From 8ff0eac590487410a41540342a5c2519a9906fed Mon Sep 17 00:00:00 2001 From: Aungkokolin1997 Date: Thu, 20 Aug 2026 04:44:49 +0000 Subject: [PATCH] [FIX] attachment_mimetype_restriction: restrict user uploads only Validating every ir.attachment creation also covered the documents Odoo writes itself: posting an invoice failed when application/xml was not allowed, and saving a report as attachment failed for application/pdf. The check now runs on the files users upload, recognized from the request carrying them, wherever the upload comes from. The policy is made explicit: the mimetype the file name declares or the one that ends up stored has to be allowed, the content has to match the magic bytes of the declared type, and a mimetype guessed from the content has to be allowed as well unless it is equivalent to the declared one (text-like types, or a zip based document detected as a plain zip). A renamed file is refused, jpeg and zip variants keep being accepted, and the verdict no longer depends on the rights of the uploader. The attachments of a posted message are filtered with the same rules, the company of the target record is read with sudo so that a portal upload does not fail on it, and the error message now names the allowed types. Assisted-by: Claude Opus 5 --- attachment_mimetype_restriction/README.rst | 21 +- .../controllers/main.py | 58 ++- .../models/ir_attachment.py | 237 ++++++++-- .../models/mail_thread.py | 179 +++++--- .../readme/DESCRIPTION.rst | 19 +- .../static/description/index.html | 18 +- .../test_attachment_mimetype_restriction.py | 414 ++++++++++++++++-- 7 files changed, 773 insertions(+), 173 deletions(-) diff --git a/attachment_mimetype_restriction/README.rst b/attachment_mimetype_restriction/README.rst index f8cb2b5ca0..490746d61d 100644 --- a/attachment_mimetype_restriction/README.rst +++ b/attachment_mimetype_restriction/README.rst @@ -32,15 +32,18 @@ Attachment MIME Type Restriction |badge1| |badge2| |badge3| |badge4| |badge5| -This module restricts attachment uploads to an explicit allowlist of MIME types -using content-based detection rather than filename extensions. Only configured -MIME types are accepted; everything else is rejected. Leaving the allowlist -empty disables the restriction and allows all file types. - -For incoming emails, the email itself is always accepted, but any attachments -whose MIME type is not in the allowlist are stripped out before the message is -saved. A security notice is then posted on the related record listing the -removed files, so users can see what was filtered. +This module restricts the files users can upload as attachments to an allowlist +of MIME types, configured company-wide and optionally per model. Leaving the +allowlist empty disables the restriction. As the MIME type Odoo derives from a +file name cannot be trusted on its own, the content of the file is checked +against it as well, so that a renamed file is rejected. + +Only what users upload is restricted. The documents the server writes on its +own, such as the pdf of a report or the xml of an electronic invoice, are not. + +For incoming emails, the email itself is always accepted, but attachments whose +MIME type is not allowed are stripped out and listed in a notice posted on the +related record. **Table of contents** diff --git a/attachment_mimetype_restriction/controllers/main.py b/attachment_mimetype_restriction/controllers/main.py index ca9650ee8c..7327488521 100644 --- a/attachment_mimetype_restriction/controllers/main.py +++ b/attachment_mimetype_restriction/controllers/main.py @@ -10,37 +10,67 @@ from odoo.addons.mail.controllers.discuss import DiscussController from odoo.addons.web.controllers.main import Binary +UPLOAD_CALLBACK_TEMPLATE = """""" + + +def _to_res_id(value): + return int(value) if str(value).isdigit() else False + class BinaryExtended(Binary): @http.route() def upload_attachment(self, model, id, ufile, callback=None): - response = super().upload_attachment(model, id, ufile, callback) - mimetype_error = getattr(request, "mimetype_error", None) - if mimetype_error: - data = response.get_data(as_text=True) - response.set_data( - data.replace( - json.dumps(_("Something horrible happened")), - json.dumps(mimetype_error), - 1, + # The whole upload is refused as soon as one of its files is, so that + # the user is told about it instead of silently losing the file. + errors = {} + files = request.httprequest.files.getlist("ufile") + for upload in files: + content = upload.read() + upload.seek(0) + try: + request.env["ir.attachment"]._check_upload_mimetype( + upload.filename, content, model, _to_res_id(id) ) - ) - return response + except ValidationError as e: + errors[upload.filename] = str(e.args[0]) if e.args else str(e) + if not errors: + return super().upload_attachment(model, id, ufile, callback) + args = [ + { + "error": errors.get( + upload.filename, + _("Not uploaded: another file of this upload was refused."), + ) + } + for upload in files + ] + if callback: + return UPLOAD_CALLBACK_TEMPLATE % (json.dumps(callback), json.dumps(args)) + return json.dumps(args) class DiscussControllerExtended(DiscussController): - @http.route("/mail/attachment/upload", methods=["POST"], type="http", auth="public") + @http.route() def mail_attachment_upload( self, ufile, thread_id, thread_model, is_pending=False, **kwargs ): try: + # Pending uploads are created on mail.compose.message, so the + # allowlist of the actual thread model has to be checked here. + content = ufile.read() + ufile.seek(0) + request.env["ir.attachment"]._check_upload_mimetype( + ufile.filename, content, thread_model, _to_res_id(thread_id) + ) return super().mail_attachment_upload( ufile, thread_id, thread_model, is_pending, **kwargs ) except ValidationError as e: error_msg = str(e.args[0]) if e.args else str(e) - attachmentData = {"error": error_msg} return request.make_response( - data=json.dumps(attachmentData), + data=json.dumps({"error": error_msg}), headers=[("Content-Type", "application/json")], ) diff --git a/attachment_mimetype_restriction/models/ir_attachment.py b/attachment_mimetype_restriction/models/ir_attachment.py index ecc751e1dc..9398c41db7 100644 --- a/attachment_mimetype_restriction/models/ir_attachment.py +++ b/attachment_mimetype_restriction/models/ir_attachment.py @@ -1,9 +1,43 @@ # Copyright 2026 Quartile (https://www.quartile.co) # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html). +import base64 +import binascii + from odoo import _, api, models from odoo.exceptions import ValidationError from odoo.http import request +from odoo.tools.mimetypes import guess_mimetype + +TEXT_LIKE_MIMETYPES = ( + "application/ecmascript", + "application/javascript", + "application/json", + "application/xml", + "image/svg", + "image/svg+xml", +) +MIMETYPE_SIGNATURES = { + "application/msword": (b"\xd0\xcf\x11\xe0\xa1\xb1\x1a\xe1",), + "application/pdf": (b"%PDF",), + "application/vnd.ms-excel": (b"\xd0\xcf\x11\xe0\xa1\xb1\x1a\xe1",), + "application/vnd.ms-powerpoint": (b"\xd0\xcf\x11\xe0\xa1\xb1\x1a\xe1",), + "image/bmp": (b"BM",), + "image/gif": (b"GIF87a", b"GIF89a"), + "image/jpeg": (b"\xff\xd8\xff",), + "image/png": (b"\x89PNG\r\n\x1a\n",), + "image/tiff": (b"II*\x00", b"MM\x00*"), + "image/x-icon": (b"\x00\x00\x01\x00",), +} +ZIP_SIGNATURES = (b"PK\x03\x04", b"PK\x05\x06", b"PK\x07\x08") +# Upload endpoints carrying the file in the json payload instead of a +# multipart form, and therefore not detected as a file upload otherwise. +JSON_UPLOAD_ENDPOINTS = ("/web_editor/attachment/add_data",) +ZIP_BASED_MIMETYPES = ( + "application/vnd.oasis.opendocument.", + "application/vnd.openxmlformats-officedocument.", + "application/zip", +) class IrAttachment(models.Model): @@ -12,9 +46,7 @@ class IrAttachment(models.Model): @api.model def _get_allowed_mimetypes(self, company_id, res_model=None): if res_model: - model = ( - self.env["ir.model"].sudo().search([("model", "=", res_model)], limit=1) - ) + model = self.env["ir.model"]._get(res_model) if model and model.attachment_allowed_mimetypes: return [ mt.strip().lower() @@ -28,62 +60,199 @@ def _get_allowed_mimetypes(self, company_id, res_model=None): return [mt.strip().lower() for mt in global_mimetypes.split(",") if mt.strip()] @api.model - def _resolve_attachment_company_id(self, vals): - company_id = vals.get("company_id") - if company_id: - return company_id - res_model = vals.get("res_model") - res_id = vals.get("res_id") + def _resolve_attachment_company_id(self, res_model=None, res_id=None): if res_model and res_id and res_model in self.env: record = self.env[res_model].sudo().browse(res_id).exists() if record and "company_id" in record._fields and record.company_id: return record.company_id.id return self.env.company.id - def _validate_mimetype_from_vals(self, vals): - if self.env.context.get("install_mode"): - return - # Skip framework-generated assets: compiled bundles (detected at create - # by res_model='ir.ui.view' + public=True, since their /web/assets/ url - # is only set in a later write) and customized scss/js overrides (which - # set url at create time). - if (vals.get("res_model") == "ir.ui.view" and vals.get("public")) or vals.get( - "url" + @api.model + def _is_text_like_mimetype(self, mimetype): + return mimetype.startswith("text/") or mimetype in TEXT_LIKE_MIMETYPES + + @api.model + def _get_stored_mimetype(self, vals): + """Return the mimetype that ir.attachment will actually store, which is + the one _check_contents computes, including its html/xml neutralization. + """ + values = self.with_context(image_no_postprocess=True)._check_contents( + dict(vals) + ) + return values["mimetype"] + + @api.model + def _get_raw_content(self, vals): + raw = vals.get("raw") + if raw: + return raw + if not vals.get("datas"): + return False + try: + return base64.b64decode(vals["datas"]) + except (binascii.Error, TypeError, ValueError): + return False + + @api.model + def _is_content_consistent(self, mimetype, raw): + """Check the content against the magic bytes of its own mimetype, as the + latter is derived from the file name whenever it provides one. + """ + if mimetype.startswith(ZIP_BASED_MIMETYPES): + return raw.startswith(ZIP_SIGNATURES) + if mimetype == "image/svg+xml": + return b"%s %s" + % (escape(blocked["name"]), escape(blocked["reason"])) + for blocked in blocked_attachments + ) + notification_body = ( + '
' + "

%s

" + "

%s

" + "
    %s
" + "

%s

" + "
" + ) % ( + escape(_("Security Notice: Blocked Attachments")), + escape(_("The following attachment(s) were blocked:")), + blocked_list, + escape( + _( + "These file types are not allowed by your organization's " + "security policy." + ) + ), + ) + try: + target_record.sudo().message_post( + body=notification_body, + message_type="notification", + subtype_xmlid="mail.mt_note", + ) + except Exception as e: + _logger.warning("Could not post blocked attachment notification: %s", e) + def _message_post_process_attachments( self, attachments, attachment_ids, message_values ): @@ -62,67 +149,31 @@ def _message_post_process_attachments( target_record = None if model and res_id and model in self.env: target_record = self.env[model].browse(res_id).exists() or None - if ( - target_record - and "company_id" in target_record._fields - and target_record.company_id - ): - company_id = target_record.company_id.id - else: - company_id = self.env.company.id - blocked_attachments_info = [] - if attachments: - filtered_attachments = [] - for attachment in attachments: - if len(attachment) == 2: - name, content = attachment - info = {} - elif len(attachment) == 3: - name, content, info = attachment - else: - continue - blocked_info = self._evaluate_attachment_against_allowlist( - name, content, info, model, res_id, company_id - ) - if blocked_info: - blocked_attachments_info.append(blocked_info) - continue - filtered_attachments.append(attachment) - attachments = filtered_attachments + allowed_mimetypes = self.env["ir.attachment"]._get_allowed_mimetypes( + self._get_attachment_allowlist_company_id(target_record), model + ) + blocked_attachments = [] + if allowed_mimetypes and attachments: + attachments, blocked = self._filter_new_attachments( + attachments, allowed_mimetypes + ) + blocked_attachments += blocked + if allowed_mimetypes and attachment_ids: + attachment_ids, blocked = self._filter_existing_attachments( + attachment_ids, allowed_mimetypes + ) + blocked_attachments += blocked result = super()._message_post_process_attachments( attachments, attachment_ids, message_values ) - if blocked_attachments_info and not target_record: + if blocked_attachments and not target_record: _logger.warning( "Blocked %d attachment(s) but no target record to notify on " "(model=%s, res_id=%s)", - len(blocked_attachments_info), + len(blocked_attachments), model, res_id, ) - if blocked_attachments_info and target_record: - blocked_list = [] - for blocked in blocked_attachments_info: - blocked_list.append( - f"• {escape(blocked['name'])} " - f"({escape(blocked['mimetype'])})" - ) - notification_body = ( - '
' - "

⚠️ Security Notice: Blocked Attachments

" - "

The following attachment(s) from the email above were " - "blocked:

" - "

" + "
".join(blocked_list) + "

" - "

These file types are not allowed by your organization's " - "security policy.

" - "
" - ) - try: - target_record.sudo().message_post( - body=notification_body, - message_type="notification", - subtype_xmlid="mail.mt_note", - ) - except Exception as e: - _logger.warning("Could not post blocked attachment notification: %s", e) + elif blocked_attachments: + self._notify_blocked_attachments(target_record, blocked_attachments) return result diff --git a/attachment_mimetype_restriction/readme/DESCRIPTION.rst b/attachment_mimetype_restriction/readme/DESCRIPTION.rst index ff02f78f72..faca8602d3 100644 --- a/attachment_mimetype_restriction/readme/DESCRIPTION.rst +++ b/attachment_mimetype_restriction/readme/DESCRIPTION.rst @@ -1,9 +1,12 @@ -This module restricts attachment uploads to an explicit allowlist of MIME types -using content-based detection rather than filename extensions. Only configured -MIME types are accepted; everything else is rejected. Leaving the allowlist -empty disables the restriction and allows all file types. +This module restricts the files users can upload as attachments to an allowlist +of MIME types, configured company-wide and optionally per model. Leaving the +allowlist empty disables the restriction. As the MIME type Odoo derives from a +file name cannot be trusted on its own, the content of the file is checked +against it as well, so that a renamed file is rejected. -For incoming emails, the email itself is always accepted, but any attachments -whose MIME type is not in the allowlist are stripped out before the message is -saved. A security notice is then posted on the related record listing the -removed files, so users can see what was filtered. +Only what users upload is restricted. The documents the server writes on its +own, such as the pdf of a report or the xml of an electronic invoice, are not. + +For incoming emails, the email itself is always accepted, but attachments whose +MIME type is not allowed are stripped out and listed in a notice posted on the +related record. diff --git a/attachment_mimetype_restriction/static/description/index.html b/attachment_mimetype_restriction/static/description/index.html index ee8f7b308b..d40a00f798 100644 --- a/attachment_mimetype_restriction/static/description/index.html +++ b/attachment_mimetype_restriction/static/description/index.html @@ -375,14 +375,16 @@

Attachment MIME Type Restriction

!! source digest: sha256:967c51f8dcbe91237a62876f00284900256f676d2ea32722d75eb75c5830e32d !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! -->

Beta License: AGPL-3 OCA/social Translate me on Weblate Try me on Runboat

-

This module restricts attachment uploads to an explicit allowlist of MIME types -using content-based detection rather than filename extensions. Only configured -MIME types are accepted; everything else is rejected. Leaving the allowlist -empty disables the restriction and allows all file types.

-

For incoming emails, the email itself is always accepted, but any attachments -whose MIME type is not in the allowlist are stripped out before the message is -saved. A security notice is then posted on the related record listing the -removed files, so users can see what was filtered.

+

This module restricts the files users can upload as attachments to an allowlist +of MIME types, configured company-wide and optionally per model. Leaving the +allowlist empty disables the restriction. As the MIME type Odoo derives from a +file name cannot be trusted on its own, the content of the file is checked +against it as well, so that a renamed file is rejected.

+

Only what users upload is restricted. The documents the server writes on its +own, such as the pdf of a report or the xml of an electronic invoice, are not.

+

For incoming emails, the email itself is always accepted, but attachments whose +MIME type is not allowed are stripped out and listed in a notice posted on the +related record.

Table of contents

    diff --git a/attachment_mimetype_restriction/tests/test_attachment_mimetype_restriction.py b/attachment_mimetype_restriction/tests/test_attachment_mimetype_restriction.py index ccd444c5e4..63504887e6 100644 --- a/attachment_mimetype_restriction/tests/test_attachment_mimetype_restriction.py +++ b/attachment_mimetype_restriction/tests/test_attachment_mimetype_restriction.py @@ -2,9 +2,54 @@ # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html). import base64 +import io +import json +import zipfile +from unittest.mock import patch +from odoo import http from odoo.exceptions import ValidationError -from odoo.tests.common import TransactionCase +from odoo.tests.common import HttpCase, TransactionCase, tagged +from odoo.tools import mute_logger +from odoo.tools.misc import hmac + +try: + import magic # noqa: F401 + + HAS_MAGIC = True +except ImportError: + HAS_MAGIC = False + + +def _zip_content(entries=None): + buffer = io.BytesIO() + with zipfile.ZipFile(buffer, "w") as archive: + for path, content in (entries or {}).items(): + archive.writestr(path, content) + return buffer.getvalue() + + +XLSX_MIMETYPE = "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet" +TEXT = b"plain text content" +HTML = b"content" +SVG = b'' +ADOBE_JPEG = b"\xff\xd8\xff\xee\x00\x0eAdobe" + b"\x00" * 64 +BINARY = b"MZ\x90\x00\x03" + b"\x00" * 64 + b"This program cannot be run in DOS mode" +ZIP = _zip_content() +ZIP_WITH_ENTRY = _zip_content({"content.txt": "content"}) +OOXML = _zip_content( + {"[Content_Types].xml": "", "xl/workbook.xml": ""} +) +PNG_DATA = ( + b"iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mP8" + b"z8DwHwAFAAH/q842iQAAAABJRU5ErkJggg==" +) +PNG_DATA_2 = ( + b"iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR4nGP4" + b"z8DwHwAFAAH/iZk9HQAAAABJRU5ErkJggg==" +) +PNG = base64.b64decode(PNG_DATA) +WEBSITE_FORM_EMAIL_TO = "support@example.com" class TestAttachmentMimetypeRestriction(TransactionCase): @@ -16,52 +61,199 @@ def setUpClass(cls): cls.company = cls.env.company cls.partner = cls.env["res.partner"].create({"name": "Test Partner"}) cls.partner_model = cls.IrModel.search([("model", "=", "res.partner")]) + cls.internal_user = cls.env["res.users"].create( + { + "name": "Internal User", + "login": "internal_user_mimetype", + "groups_id": [(6, 0, [cls.env.ref("base.group_user").id])], + } + ) - def _create_text_attachment(self, **overrides): - vals = { - "name": "test_file.txt", - "datas": base64.b64encode(b"test data"), - } - vals.update(overrides) - return self.Attachment.create(vals) + def _policy_matrix(self): + """(label, allowlist, file name, content, as internal user, stored + mimetype or None when the upload has to be refused). + """ + return [ + ("text in .txt", "text/plain", "notes.txt", TEXT, False, "text/plain"), + ("html in .txt", "text/plain", "notes.txt", HTML, False, "text/plain"), + ("svg in .txt", "text/plain", "notes.txt", SVG, False, "text/plain"), + ("png in .txt", "text/plain", "notes.txt", PNG, False, None), + ( + "unknown binary in .txt", + "text/plain", + "notes.txt", + BINARY, + False, + None if HAS_MAGIC else "text/plain", + ), + (".svg as system", "text/plain", "logo.svg", SVG, False, None), + (".svg as user", "text/plain", "logo.svg", SVG, True, "text/plain"), + ( + "svg allowed as system", + "image/svg+xml", + "logo.svg", + SVG, + False, + "image/svg+xml", + ), + ( + "svg allowed as user", + "image/svg+xml", + "logo.svg", + SVG, + True, + "text/plain", + ), + ("png in .png", "image/png", "photo.png", PNG, False, "image/png"), + ("text in .png", "image/png", "photo.png", TEXT, False, None), + ( + "adobe jpeg in .jpg", + "image/jpeg", + "adobe.jpg", + ADOBE_JPEG, + False, + "image/jpeg", + ), + ( + "stored only zip in .zip", + "application/zip", + "archive.zip", + ZIP, + False, + "application/zip", + ), + ("ooxml in .xlsx", XLSX_MIMETYPE, "book.xlsx", OOXML, False, XLSX_MIMETYPE), + ( + "plain zip in .xlsx", + XLSX_MIMETYPE, + "book.xlsx", + ZIP_WITH_ENTRY, + False, + XLSX_MIMETYPE, + ), + ] - def test_non_allowed_mimetype_blocked(self): - self.company.attachment_allowed_mimetypes = "image/png" - with self.assertRaises(ValidationError) as cm: - self._create_text_attachment() - self.assertIn("text/plain", str(cm.exception)) + def test_mimetype_policy_matrix(self): + for label, allowed, name, content, as_user, expected in self._policy_matrix(): + with self.subTest(label): + model = self.Attachment + if as_user: + model = model.with_user(self.internal_user) + error = model._get_mimetype_error( + {"name": name, "raw": content}, allowed.split(",") + ) + if expected is None: + self.assertTrue(error, label) + continue + self.assertFalse(error, label) + attachment = model.create( + {"name": name, "datas": base64.b64encode(content)} + ).sudo() + self.assertEqual(attachment.mimetype, expected, label) - def test_allowed_mimetype_create(self): - self.company.attachment_allowed_mimetypes = "text/plain,application/pdf" - attachment = self._create_text_attachment() - self.assertEqual(attachment.mimetype, "text/plain") + def test_server_created_attachment_not_restricted(self): + self.company.attachment_allowed_mimetypes = "image/png" + attachment = self.Attachment.create( + { + "name": "INV_0001.pdf", + "raw": b"%PDF-1.4\n1 0 obj\n<<>>\nendobj\ntrailer\n%%EOF\n", + "res_model": "res.partner", + "res_id": self.partner.id, + } + ) + self.assertEqual(attachment.mimetype, "application/pdf") - def test_empty_config_allows_all(self): + def test_upload_check_without_configuration_allows_all(self): self.company.attachment_allowed_mimetypes = "" - self.assertTrue(self._create_text_attachment()) + self.Attachment._check_upload_mimetype("note.txt", TEXT) - def test_per_model_overrides_global(self): + def _as_upload(self): + return patch.object(type(self.Attachment), "_is_user_upload", lambda self: True) + + def test_write_is_restricted_for_uploads(self): self.company.attachment_allowed_mimetypes = "image/png" - self.partner_model.attachment_allowed_mimetypes = "text/plain" - attachment = self._create_text_attachment( - res_model="res.partner", res_id=self.partner.id + attachment = self.Attachment.create( + {"name": "logo.png", "datas": base64.b64encode(PNG)} ) - self.assertTrue(attachment) + with self._as_upload(): + with self.assertRaises(ValidationError): + attachment.write({"datas": base64.b64encode(TEXT)}) - def test_per_model_empty_falls_through_to_global(self): - self.company.attachment_allowed_mimetypes = "image/png" - self.partner_model.attachment_allowed_mimetypes = "" - with self.assertRaises(ValidationError): - self._create_text_attachment( - res_model="res.partner", res_id=self.partner.id + def test_binary_field_storage_not_restricted(self): + self.company.attachment_allowed_mimetypes = "text/plain" + with self._as_upload(): + self.partner.image_1920 = PNG_DATA + attachment = self.Attachment.sudo().search( + [ + ("res_model", "=", "res.partner"), + ("res_id", "=", self.partner.id), + ("res_field", "=", "image_1920"), + ] + ) + self.assertEqual(attachment.mimetype, "image/png") + + def test_accepted_attachment_survives_message_post(self): + """An attachment accepted on upload must not be stripped when posted: + its stored mimetype is text/plain while the allowlist names svg.""" + self.company.attachment_allowed_mimetypes = "image/svg+xml" + attachment = ( + self.Attachment.with_user(self.internal_user) + .create( + { + "name": "logo.svg", + "datas": base64.b64encode(SVG), + "res_model": "mail.compose.message", + "res_id": 0, + } ) + .sudo() + ) + self.assertEqual(attachment.mimetype, "text/plain") + message = self.partner.message_post( + body="

    body

    ", attachment_ids=[attachment.id] + ) + self.assertIn("logo.svg", message.attachment_ids.mapped("name")) - def test_write_revalidates_on_datas_change(self): - self.company.attachment_allowed_mimetypes = "text/plain" - attachment = self._create_text_attachment() + def test_attachment_processing_without_read_right_on_target(self): + self.company.attachment_allowed_mimetypes = "image/png" + attachment = self.Attachment.create( + {"name": "logo.png", "datas": base64.b64encode(PNG)} + ) + self.env.cache.invalidate() + result = self.partner.with_user( + self.env.ref("base.public_user") + )._message_post_process_attachments( + [], + [attachment.id], + {"res_id": self.partner.id, "model": "res.partner"}, + ) + self.assertIn("attachment_ids", result) + + def test_message_post_blocks_stored_mimetype_mismatch(self): + self.company.attachment_allowed_mimetypes = "" + attachment = self.Attachment.create( + { + "name": "photo.png", + "mimetype": "text/scss", + "datas": base64.b64encode(TEXT), + } + ) self.company.attachment_allowed_mimetypes = "image/png" + message = self.partner.message_post( + body="

    body

    ", attachment_ids=[attachment.id] + ) + self.assertNotIn("photo.png", message.attachment_ids.mapped("name")) + + def test_upload_check_uses_thread_model_allowlist(self): + self.company.attachment_allowed_mimetypes = "text/plain" + self.partner_model.attachment_allowed_mimetypes = "image/png" with self.assertRaises(ValidationError): - attachment.write({"datas": base64.b64encode(b"updated content")}) + self.Attachment._check_upload_mimetype( + "note.txt", TEXT, "res.partner", self.partner.id + ) + self.Attachment._check_upload_mimetype( + "logo.png", PNG, "res.partner", self.partner.id + ) def test_message_post_filters_blocked_attachments(self): self.company.attachment_allowed_mimetypes = "text/html" @@ -75,11 +267,161 @@ def test_message_post_filters_blocked_attachments(self): attachments = self.Attachment.search( [("res_model", "=", "res.partner"), ("res_id", "=", self.partner.id)] ) - self.assertEqual(attachments.mapped("name"), ["allowed.html"]) + self.assertEqual(set(attachments.mapped("name")), {"allowed.html"}) notice = ( self.env["mail.message"] .search([("res_id", "=", self.partner.id), ("model", "=", "res.partner")]) - .filtered(lambda m: "Security Notice" in m.body) + .filtered(lambda m: "Blocked Attachments" in m.body) ) self.assertEqual(len(notice), 1) self.assertIn("blocked.txt", notice.body) + + def test_message_post_filters_blocked_attachment_ids(self): + self.company.attachment_allowed_mimetypes = "" + txt_att = self.Attachment.create( + { + "name": "will_be_blocked.txt", + "datas": base64.b64encode(b"text content"), + } + ) + png_att = self.Attachment.create( + { + "name": "allowed.png", + "datas": base64.b64encode(base64.b64decode(PNG_DATA)), + } + ) + self.company.attachment_allowed_mimetypes = "image/png" + message = self.partner.message_post( + body="

    Test

    ", + attachment_ids=[txt_att.id, png_att.id], + ) + self.assertEqual(set(message.attachment_ids.mapped("name")), {"allowed.png"}) + notice = ( + self.env["mail.message"] + .search([("res_id", "=", self.partner.id), ("model", "=", "res.partner")]) + .filtered(lambda m: "Blocked Attachments" in m.body) + ) + self.assertEqual(len(notice), 1) + self.assertIn("will_be_blocked.txt", notice.body) + + +@tagged("post_install", "-at_install") +class TestAttachmentMimetypeRestrictionUpload(HttpCase): + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.company = cls.env.company + cls.partner = cls.env["res.partner"].create({"name": "Upload Partner"}) + cls.password = "upload_user_mimetype" + cls.user = cls.env["res.users"].create( + { + "name": "Upload User", + "login": cls.password, + "password": cls.password, + "email": "upload@example.com", + "groups_id": [ + ( + 6, + 0, + [ + cls.env.ref("base.group_user").id, + cls.env.ref("base.group_partner_manager").id, + ], + ) + ], + } + ) + + def _configure(self, allowed): + self.company.attachment_allowed_mimetypes = allowed + self.company.flush() + self.authenticate(self.password, self.password) + + def _attachment_count(self, name): + return ( + self.env["ir.attachment"] + .sudo() + .search_count([("name", "=", name), ("res_id", "=", self.partner.id)]) + ) + + def _post_chatter_upload(self, filename, content): + return self.url_open( + "/mail/attachment/upload", + data={ + "thread_id": self.partner.id, + "thread_model": "res.partner", + "csrf_token": http.WebRequest.csrf_token(self), + }, + files=[("ufile", (filename, content, "application/octet-stream"))], + ) + + def _post_record_upload(self, filename, content): + return self.url_open( + "/web/binary/upload_attachment", + data={ + "model": "res.partner", + "id": self.partner.id, + "csrf_token": http.WebRequest.csrf_token(self), + }, + files=[("ufile", (filename, content, "application/octet-stream"))], + ) + + def test_chatter_upload_is_restricted(self): + self._configure("image/png") + refused = self._post_chatter_upload("note.txt", TEXT) + self.assertIn("is not allowed", refused.text) + self.assertFalse(self._attachment_count("note.txt")) + accepted = self._post_chatter_upload("logo.png", PNG) + self.assertNotIn("is not allowed", accepted.text) + self.assertTrue(self._attachment_count("logo.png")) + + def test_record_upload_is_restricted(self): + self._configure("image/png") + refused = self._post_record_upload("note.txt", TEXT) + self.assertIn("is not allowed", refused.text) + self.assertFalse(self._attachment_count("note.txt")) + accepted = self._post_record_upload("logo.png", PNG) + self.assertNotIn("is not allowed", accepted.text) + self.assertTrue(self._attachment_count("logo.png")) + + def _submit_website_form(self, filename, content): + return self.url_open( + "/website/form/mail.mail", + data={ + "email_from": "visitor@example.com", + "email_to": WEBSITE_FORM_EMAIL_TO, + "subject": "Website form probe", + "body_html": "

    question

    ", + "website_form_signature": hmac( + self.env, "website_form_signature", WEBSITE_FORM_EMAIL_TO + ), + }, + files=[("attachment", (filename, content, "application/octet-stream"))], + ) + + @mute_logger("odoo.addons.mail.models.mail_mail") + def test_website_form_upload_is_restricted(self): + """The website form is the upload path of, among others, a helpdesk + ticket form, and it is covered without depending on website.""" + if not self.env["ir.module.module"].search( + [("name", "=", "website"), ("state", "=", "installed")] + ): + self.skipTest("website is not installed") + self.company.attachment_allowed_mimetypes = "image/png" + self.company.flush() + # the website form is a visitor path: an authenticated session would + # make core require a csrf token + self.authenticate(None, None) + response = self._submit_website_form("virus.txt", TEXT) + self.assertEqual(response.status_code, 200) + self.assertIn("is not allowed", json.loads(response.text).get("error", "")) + self.assertFalse( + self.env["ir.attachment"].sudo().search_count([("name", "=", "virus.txt")]) + ) + response = self._submit_website_form("logo.png", PNG) + self.assertNotIn("error", json.loads(response.text)) + self.assertTrue( + self.env["ir.attachment"] + .sudo() + .search_count([("name", "=", "logo.png"), ("res_model", "=", "mail.mail")]) + )