From 65af112e2b50f03098adedd3bed672b5a7cff273 Mon Sep 17 00:00:00 2001 From: Laurent Smet Date: Wed, 25 Jan 2023 11:56:20 +0000 Subject: [PATCH] [FIX] account_edi*: Fix access rights for account.edi.document.attachment_id field An attachment has complex access rights. If there is no res_model/res_id, the access rights are admin (not exactly but let's say that). When an EDI like Facturx/E-FFF generates an attachment not linked to any model, you don't have access to it except if you are admin. However, here we have a security issue since everyone is able to write any 'id' on the 'attachment_id' field. If you do that using Facturx, knowing this EDI will embed its attachment inside the invoice PDF report in sudo mode, you have now a way to extract any attachment from the database including the ones you shouldn't have access to. Furthermore, a different api introduced by OWL makes the form view of account.edi.document popping from the one2many inside the invoice form. Instead of "options={'no_open': '1'}", the new api is now to put directly "no_open='1'" on the root node. If you combine both issues above, you currently have a way to extract any 'attachment_id' from the database and odoo is kind enough to give you the form view to do it. closes odoo/odoo#111210 Solution: "account.edi.document.attachment_id" is now accessible to the admin only. X-original-commit: 44a4cdb3944a4b722dcfbca5e2947a4372b8501d Signed-off-by: Olivier Colson (oco) --- addons/account_edi/models/account_edi_document.py | 14 +++++++++----- addons/account_edi/models/ir_actions_report.py | 2 +- addons/account_edi/views/account_move_views.xml | 4 ++-- .../models/account_edi_format.py | 5 +++-- .../models/ir_actions_report.py | 4 ++-- addons/l10n_it_edi/models/account_edi_format.py | 5 +++-- 6 files changed, 20 insertions(+), 14 deletions(-) diff --git a/addons/account_edi/models/account_edi_document.py b/addons/account_edi/models/account_edi_document.py index bf807d36b16..40fa64a37fc 100644 --- a/addons/account_edi/models/account_edi_document.py +++ b/addons/account_edi/models/account_edi_document.py @@ -20,7 +20,11 @@ class AccountEdiDocument(models.Model): # == Stored fields == move_id = fields.Many2one('account.move', required=True, ondelete='cascade') edi_format_id = fields.Many2one('account.edi.format', required=True) - attachment_id = fields.Many2one('ir.attachment', help='The file generated by edi_format_id when the invoice is posted (and this document is processed).') + attachment_id = fields.Many2one( + comodel_name='ir.attachment', + groups='base.group_system', + help="The file generated by edi_format_id when the invoice is posted (and this document is processed).", + ) state = fields.Selection([('to_send', 'To Send'), ('sent', 'Sent'), ('to_cancel', 'To Cancel'), ('cancelled', 'Cancelled')]) error = fields.Html(help='The text of the last error that happened during Electronic Invoice operation.') blocking_level = fields.Selection( @@ -116,8 +120,8 @@ class AccountEdiDocument(models.Model): move = document.move_id move_result = edi_result.get(move, {}) if move_result.get('attachment'): - old_attachment = document.attachment_id - document.attachment_id = move_result['attachment'] + old_attachment = document.sudo().attachment_id + document.sudo().attachment_id = move_result['attachment'] if not old_attachment.res_model or not old_attachment.res_id: attachments_to_unlink |= old_attachment if move_result.get('success') is True: @@ -144,7 +148,7 @@ class AccountEdiDocument(models.Model): move_result = edi_result.get(move, {}) if move_result.get('success') is True: old_attachment = document.sudo().attachment_id - document.write({ + document.sudo().write({ 'state': 'cancelled', 'error': False, 'attachment_id': False, @@ -218,7 +222,7 @@ class AccountEdiDocument(models.Model): for job in jobs_to_process: documents = job['documents'] move_to_lock = documents.move_id - attachments_potential_unlink = documents.attachment_id.filtered(lambda a: not a.res_model and not a.res_id) + attachments_potential_unlink = documents.sudo().attachment_id.filtered(lambda a: not a.res_model and not a.res_id) try: with self.env.cr.savepoint(flush=False): self._cr.execute('SELECT * FROM account_edi_document WHERE id IN %s FOR UPDATE NOWAIT', [tuple(documents.ids)]) diff --git a/addons/account_edi/models/ir_actions_report.py b/addons/account_edi/models/ir_actions_report.py index 45da8ac3eba..45ad7ba007a 100644 --- a/addons/account_edi/models/ir_actions_report.py +++ b/addons/account_edi/models/ir_actions_report.py @@ -36,7 +36,7 @@ class IrActionsReport(models.Model): # The attachements on the edi documents are only system readable # because they don't have res_id and res_model, here we are sure that # the user has access to the invoice and edi document - edi_document.edi_format_id._prepare_invoice_report(writer, edi_document.sudo()) + edi_document.edi_format_id._prepare_invoice_report(writer, edi_document) # Replace the current content. pdf_stream.close() diff --git a/addons/account_edi/views/account_move_views.xml b/addons/account_edi/views/account_move_views.xml index 7c12371ff55..8e7ed38f13f 100644 --- a/addons/account_edi/views/account_move_views.xml +++ b/addons/account_edi/views/account_move_views.xml @@ -102,8 +102,8 @@ string="EDI Documents" groups="base.group_no_one" attrs="{'invisible': [('edi_document_ids', '=', [])]}"> - - + + diff --git a/addons/account_edi_ubl_cii/models/account_edi_format.py b/addons/account_edi_ubl_cii/models/account_edi_format.py index ecdd11a974b..6c7333bda54 100644 --- a/addons/account_edi_ubl_cii/models/account_edi_format.py +++ b/addons/account_edi_ubl_cii/models/account_edi_format.py @@ -151,10 +151,11 @@ class AccountEdiFormat(models.Model): self.ensure_one() if self.code != 'facturx_1_0_05': return super()._prepare_invoice_report(pdf_writer, edi_document) - if not edi_document.attachment_id: + attachment = edi_document.sudo().attachment_id + if not attachment: return - pdf_writer.embed_odoo_attachment(edi_document.attachment_id, subtype='text/xml') + pdf_writer.embed_odoo_attachment(attachment, subtype='text/xml') if not pdf_writer.is_pdfa: try: pdf_writer.convert_to_pdfa() diff --git a/addons/account_edi_ubl_cii/models/ir_actions_report.py b/addons/account_edi_ubl_cii/models/ir_actions_report.py index ddca3dd8292..5ef129eb0a0 100644 --- a/addons/account_edi_ubl_cii/models/ir_actions_report.py +++ b/addons/account_edi_ubl_cii/models/ir_actions_report.py @@ -16,7 +16,7 @@ class IrActionsReport(models.Model): def _add_pdf_into_invoice_xml(self, invoice, stream_data): format_codes = ['ubl_bis3', 'ubl_de', 'nlcius_1', 'efff_1'] - edi_attachments = invoice.edi_document_ids.filtered(lambda d: d.edi_format_id.code in format_codes).attachment_id + edi_attachments = invoice.edi_document_ids.filtered(lambda d: d.edi_format_id.code in format_codes).sudo().attachment_id for edi_attachment in edi_attachments: old_xml = base64.b64decode(edi_attachment.with_context(bin_size=False).datas, validate=True) tree = etree.fromstring(old_xml) @@ -45,7 +45,7 @@ class IrActionsReport(models.Model): anchor_index = tree.index(anchor_elements[0]) tree.insert(anchor_index, etree.fromstring(to_inject)) new_xml = etree.tostring(cleanup_xml_node(tree)) - edi_attachment.write({ + edi_attachment.sudo().write({ 'res_model': 'account.move', 'res_id': invoice.id, 'datas': base64.b64encode(new_xml), diff --git a/addons/l10n_it_edi/models/account_edi_format.py b/addons/l10n_it_edi/models/account_edi_format.py index 91244cc448d..291dc97c94c 100644 --- a/addons/l10n_it_edi/models/account_edi_format.py +++ b/addons/l10n_it_edi/models/account_edi_format.py @@ -817,8 +817,9 @@ class AccountEdiFormat(models.Model): self.ensure_one() if self.code != 'fattura_pa': return super()._prepare_invoice_report(pdf_writer, edi_document) - if edi_document.attachment_id: - pdf_writer.embed_odoo_attachment(edi_document.attachment_id) + attachment = edi_document.sudo().attachment_id + if attachment: + pdf_writer.embed_odoo_attachment(attachment) def _is_compatible_with_journal(self, journal): # OVERRIDE