From b23cb16487e2bc054c81ee07ede5460de8409493 Mon Sep 17 00:00:00 2001 From: Denis Ledoux Date: Wed, 6 Dec 2023 14:38:17 +0100 Subject: [PATCH] [FIX] core: restrict httprequest attributes The version of werkzeug installed can vary from one deployment to another, as we recommend to use the operating system package, and the version can therefore change according to the operating system version. e.g. the werkzeug version installed using `apt install python3-werkzeug` varies between Ubuntu 18.04, 20.04, 22.04, 23.10, ... We want to keep under control the attributes developers use on werkzeug.wrappers.Request, to avoid compatibility issues from one version to another. Therefore, this revision aims to subclass werkzeug.wrappers.Request to limit the attributes which can be used. task-3734305 Part-of: odoo/odoo#78857 Signed-off-by: Julien Castiaux (juc) --- addons/bus/websocket.py | 2 +- addons/http_routing/models/ir_http.py | 8 ++-- odoo/addons/test_http/controllers.py | 11 +++++ odoo/addons/test_http/tests/__init__.py | 1 + odoo/addons/test_http/tests/test_security.py | 17 +++++++ odoo/http.py | 49 ++++++++++++++++++-- 6 files changed, 77 insertions(+), 11 deletions(-) create mode 100644 odoo/addons/test_http/tests/test_security.py diff --git a/addons/bus/websocket.py b/addons/bus/websocket.py index 1775b9b47a5..e2c899da423 100644 --- a/addons/bus/websocket.py +++ b/addons/bus/websocket.py @@ -833,7 +833,7 @@ class WebsocketConnectionHandler: cls._handle_public_configuration(request) try: response = cls._get_handshake_response(request.httprequest.headers) - socket = request.httprequest.environ['socket'] + socket = request.httprequest._HTTPRequest__environ['socket'] session, db, httprequest = request.session, request.db, request.httprequest response.call_on_close(lambda: cls._serve_forever( Websocket(socket, session), diff --git a/addons/http_routing/models/ir_http.py b/addons/http_routing/models/ir_http.py index b2a0ae131f3..8fa6d55fbe6 100644 --- a/addons/http_routing/models/ir_http.py +++ b/addons/http_routing/models/ir_http.py @@ -23,7 +23,7 @@ from odoo import api, models, exceptions, tools, http from odoo.addons.base.models import ir_http from odoo.addons.base.models.ir_http import RequestUID from odoo.addons.base.models.ir_qweb import QWebException -from odoo.http import request, Response +from odoo.http import request, HTTPRequest, Response from odoo.osv import expression from odoo.tools import config, ustr, pycompat @@ -527,16 +527,14 @@ class IrHttp(models.AbstractModel): query_string = request.httprequest.environ['QUERY_STRING'] # Change the WSGI environment - environ = request.httprequest.environ.copy() + environ = request.httprequest._HTTPRequest__environ.copy() environ['PATH_INFO'] = path environ['QUERY_STRING'] = query_string environ['RAW_URI'] = f'{path}?{query_string}' # REQUEST_URI left as-is so it still contains the original URI # Create and expose a new request from the modified WSGI env - httprequest = werkzeug.wrappers.Request(environ) - httprequest.parameter_storage_class = ( - werkzeug.datastructures.ImmutableOrderedMultiDict) + httprequest = HTTPRequest(environ) threading.current_thread().url = httprequest.url request.httprequest = httprequest diff --git a/odoo/addons/test_http/controllers.py b/odoo/addons/test_http/controllers.py index 4da4da7ea73..41b077d6047 100644 --- a/odoo/addons/test_http/controllers.py +++ b/odoo/addons/test_http/controllers.py @@ -210,3 +210,14 @@ class TestHttp(http.Controller): raise SerializationFailureError() return data.decode() + + # ===================================================== + # Security + # ===================================================== + @http.route('/test_http/httprequest_attrs', type='http', auth='none') + def request_attrs(self): + return json.dumps(dir(request.httprequest)) + + @http.route('/test_http/httprequest_environ', type='http', auth='none') + def request_environ(self): + return json.dumps(list(request.httprequest.environ.keys())) diff --git a/odoo/addons/test_http/tests/__init__.py b/odoo/addons/test_http/tests/__init__.py index cc771f36c03..ea5bc97480c 100644 --- a/odoo/addons/test_http/tests/__init__.py +++ b/odoo/addons/test_http/tests/__init__.py @@ -4,6 +4,7 @@ from . import test_error from . import test_greeting from . import test_misc from . import test_models +from . import test_security from . import test_session from . import test_static from . import test_web_server diff --git a/odoo/addons/test_http/tests/test_security.py b/odoo/addons/test_http/tests/test_security.py new file mode 100644 index 00000000000..4dc0bca76e0 --- /dev/null +++ b/odoo/addons/test_http/tests/test_security.py @@ -0,0 +1,17 @@ +import json +from .test_common import TestHttpBase + + +class TestHttpSecurity(TestHttpBase): + def test_httprequest_attrs(self): + res = self.db_url_open('/test_http/httprequest_attrs') + result = json.loads(res.content) + self.assertNotIn('user_agent_class', result) + self.assertNotIn('parameter_storage_class', result) + + def test_httprequest_environ(self): + res = self.db_url_open('/test_http/httprequest_environ') + result = json.loads(res.content) + self.assertNotIn('wsgi.input', result) + self.assertNotIn('werkzeug.socket', result) + self.assertNotIn('socket', result) diff --git a/odoo/http.py b/odoo/http.py index 32ad8b31ade..3da421e8df0 100644 --- a/odoo/http.py +++ b/odoo/http.py @@ -1165,6 +1165,49 @@ def borrow_request(): _request_stack.push(req) +def make_request_wrap_methods(attr): + def getter(self): + return getattr(self._HTTPRequest__wrapped, attr) + + def setter(self, value): + return setattr(self._HTTPRequest__wrapped, attr, value) + + return getter, setter + + +class HTTPRequest: + def __init__(self, environ): + httprequest = werkzeug.wrappers.Request(environ) + 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 + + self.__wrapped = httprequest + self.__environ = self.__wrapped.environ + self.environ = { + key: value + for key, value in self.__environ.items() + if (not key.startswith(('werkzeug.', 'wsgi.', 'socket')) or key in ['wsgi.url_scheme']) + } + + def __enter__(self): + return self + + +HTTPREQUEST_ATTRIBUTES = [ + '__str__', '__repr__', '__exit__', + 'accept_charsets', 'accept_languages', 'accept_mimetypes', 'access_route', 'args', 'authorization', 'base_url', + 'charset', 'content_encoding', 'content_length', 'content_md5', 'content_type', 'cookies', 'data', 'date', + 'encoding_errors', 'files', 'form', 'full_path', 'get_data', 'get_json', 'headers', 'host', 'host_url', 'if_match', + 'if_modified_since', 'if_none_match', 'if_range', 'if_unmodified_since', 'is_json', 'is_secure', 'json', + 'max_content_length', 'method', 'mimetype', 'mimetype_params', 'origin', 'path', 'pragma', 'query_string', 'range', + 'referrer', 'remote_addr', 'remote_user', 'root_path', 'root_url', 'scheme', 'script_root', 'server', 'session', + 'trusted_hosts', 'url', 'url_charset', 'url_root', 'user_agent', 'values', +] +for attr in HTTPREQUEST_ATTRIBUTES: + setattr(HTTPRequest, attr, property(*make_request_wrap_methods(attr))) + + class Response(werkzeug.wrappers.Response): """ Outgoing HTTP response with body, status, headers and qweb support. @@ -2132,11 +2175,7 @@ class Application: return ProxyFix(fake_app)(environ, fake_start_response) - with werkzeug.wrappers.Request(environ) as httprequest: - 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 + with HTTPRequest(environ) as httprequest: request = Request(httprequest) _request_stack.push(request) request._post_init()