From cc5a14b6a9243dc56b9011ea16634a07aff585b9 Mon Sep 17 00:00:00 2001 From: Julien Castiaux Date: Thu, 29 Jun 2023 13:45:06 +0000 Subject: [PATCH] [FIX] core: prevent upload of large files The limit was only enforced by the front-end, meaning that anybody could forge a request with a huge file and get it processed by Odoo. According to the documentation of werkzeug[^1], such limit should be enforced by the server server instead of the wsgi application. It is the case for Odoo Online but on-premise customers might not configure their servers. The `web.max_file_upload_size` system paramter is now enforced upon parsing the content of the request. It defaults at 128 MiB which is enough for most documents and images. We do not want to host large files (e.g. videos) in the Odoo filestore. [^1]: https://werkzeug.palletsprojects.com/en/2.0.x/request_data/ Fixes: #124646 Part-of: odoo/odoo#126914 --- addons/web/controllers/binary.py | 4 +- addons/web/models/ir_http.py | 4 +- odoo/addons/test_http/tests/test_static.py | 64 +++++++++++++++++++++- odoo/http.py | 15 +++++ 4 files changed, 82 insertions(+), 5 deletions(-) diff --git a/addons/web/controllers/binary.py b/addons/web/controllers/binary.py index 7571edb0076..08a1462e3b0 100644 --- a/addons/web/controllers/binary.py +++ b/addons/web/controllers/binary.py @@ -187,7 +187,7 @@ class Binary(http.Controller): try: attachment = Model.create({ 'name': filename, - 'datas': base64.encodebytes(ufile.read()), + 'raw': ufile.read(), 'res_model': model, 'res_id': int(id) }) @@ -200,7 +200,7 @@ class Binary(http.Controller): else: args.append({ 'filename': clean(filename), - 'mimetype': ufile.content_type, + 'mimetype': attachment.mimetype, 'id': attachment.id, 'size': attachment.file_size }) diff --git a/addons/web/models/ir_http.py b/addons/web/models/ir_http.py index 331a7852e66..21cfe26c05b 100644 --- a/addons/web/models/ir_http.py +++ b/addons/web/models/ir_http.py @@ -6,7 +6,7 @@ import logging import odoo from odoo import api, http, models -from odoo.http import request +from odoo.http import request, DEFAULT_MAX_CONTENT_LENGTH from odoo.tools import file_open, image_process, ustr from odoo.tools.misc import str2bool @@ -84,7 +84,7 @@ class Http(models.AbstractModel): IrConfigSudo = self.env['ir.config_parameter'].sudo() max_file_upload_size = int(IrConfigSudo.get_param( 'web.max_file_upload_size', - default=128 * 1024 * 1024, # 128MiB + default=DEFAULT_MAX_CONTENT_LENGTH, )) mods = odoo.conf.server_wide_modules or [] if request.db: diff --git a/odoo/addons/test_http/tests/test_static.py b/odoo/addons/test_http/tests/test_static.py index f32032564a8..6181023612f 100644 --- a/odoo/addons/test_http/tests/test_static.py +++ b/odoo/addons/test_http/tests/test_static.py @@ -2,12 +2,13 @@ import base64 from datetime import datetime, timedelta +from http import HTTPStatus from os.path import basename, join as opj from unittest.mock import patch from freezegun import freeze_time import odoo -from odoo.tests import new_test_user, tagged +from odoo.tests import new_test_user, tagged, RecordCapturer from odoo.tools import config, file_open, image_process from .test_common import TestHttpBase @@ -395,6 +396,7 @@ class TestHttpStaticLogo(TestHttpStaticCommon): self.assertDownloadLogoDefault(company=self.company2, user=self.user_of_company_of_superuser) +@tagged('post_install', '-at_install') class TestHttpStaticCache(TestHttpStaticCommon): @freeze_time(datetime.utcnow()) def test_static_cache0_standard(self, domain=''): @@ -448,3 +450,63 @@ class TestHttpStaticCache(TestHttpStaticCommon): }) res2.raise_for_status() self.assertEqual(res2.status_code, 304, "We should not download the file again.") + + +@tagged('post_install', '-at_install') +class TestHttpStaticUpload(TestHttpStaticCommon): + def test_upload_small_file(self): + new_test_user(self.env, 'jackoneill') + self.authenticate('jackoneill', 'jackoneill') + + with RecordCapturer(self.env['ir.attachment'], []) as capture, \ + file_open('test_http/static/src/img/gizeh.png', 'rb') as file: + file_content = file.read() + file_size = len(file_content) + file.seek(0) + res = self.opener.post( + f'{self.base_url()}/web/binary/upload_attachment', + files={'ufile': file}, + data={ + 'csrf_token': odoo.http.Request.csrf_token(self), + 'model': 'test_http.stargate', + 'id': self.env.ref('test_http.earth').id, + }, + ) + res.raise_for_status() + + self.assertEqual(len(capture.records), 1, "An attachment should have been created") + self.assertEqual(capture.records.name, 'gizeh.png') + self.assertEqual(capture.records.raw, file_content) + self.assertEqual(capture.records.mimetype, 'image/png') + + self.assertEqual(res.json(), [{ + 'filename': 'gizeh.png', + 'mimetype': 'image/png', + 'id': capture.records.id, + 'size': file_size, + }]) + + + def test_upload_large_file(self): + new_test_user(self.env, 'jackoneill') + self.authenticate('jackoneill', 'jackoneill') + + with RecordCapturer(self.env['ir.attachment'], []) as capture, \ + file_open('test_http/static/src/img/gizeh.png', 'rb') as file: + file_size = file.seek(0, 2) + file.seek(0) + self.env['ir.config_parameter'].sudo().set_param( + 'web.max_file_upload_size', file_size - 1, + ) + res = self.opener.post( + f'{self.base_url()}/web/binary/upload_attachment', + files={'ufile': file}, + data={ + 'csrf_token': odoo.http.Request.csrf_token(self), + 'model': 'test_http.stargate', + 'id': self.env.ref('test_http.earth').id, + 'callback': 'callmemaybe', + }, + ) + self.assertFalse(capture.records, "No attachment should have been created") + self.assertEqual(res.status_code, HTTPStatus.REQUEST_ENTITY_TOO_LARGE) diff --git a/odoo/http.py b/odoo/http.py index 0bc5c7284d7..3275be8611b 100644 --- a/odoo/http.py +++ b/odoo/http.py @@ -232,6 +232,8 @@ def get_default_session(): 'session_token': None, } +DEFAULT_MAX_CONTENT_LENGTH = 128 * 1024 * 1024 # 128MiB + # Two empty objects used when the geolocalization failed. They have the # sames attributes as real countries/cities except that accessing them # evaluates to None. @@ -1482,6 +1484,18 @@ class Request: :returns: The merged key-value pairs. :rtype: dict """ + if self.env: + ICP = self.env['ir.config_parameter'].sudo() + try: + key = 'web.max_file_upload_size' + self.httprequest.max_content_length = int(ICP.get_param( + key, DEFAULT_MAX_CONTENT_LENGTH + )) + except ValueError: # better not crash on ALL requests + _logger.error("invalid %s: %r, use %s instead", + key, ICP.get_param(key), self.httprequest.max_content_length, + ) + params = { **self.httprequest.args, **self.httprequest.form, @@ -2121,6 +2135,7 @@ class Application: httprequest.user_agent_class = UserAgent # use vendored userAgent since it will be removed in 2.1 httprequest.parameter_storage_class = ( werkzeug.datastructures.ImmutableOrderedMultiDict) + httprequest.max_content_length = DEFAULT_MAX_CONTENT_LENGTH request = Request(httprequest) _request_stack.push(request) request._post_init()