From ab7c0b4f365a2637da0367884538909fb5e06db4 Mon Sep 17 00:00:00 2001 From: "Laurent Mignon (ACSONE)" Date: Wed, 8 Mar 2017 13:37:33 +0100 Subject: [PATCH] [IMP] report_py3o: prevent injections when retrieving the template from path --- report_py3o/README.rst | 30 +++++++ report_py3o/models/py3o_report.py | 51 +++++++++-- report_py3o/tests/test_report_py3o.py | 124 +++++++++++++++++++------- 3 files changed, 163 insertions(+), 42 deletions(-) diff --git a/report_py3o/README.rst b/report_py3o/README.rst index 58731bec..d29fcdef 100644 --- a/report_py3o/README.rst +++ b/report_py3o/README.rst @@ -127,6 +127,36 @@ For example, to replace the native invoice report by a custom py3o report, add t where *my_custom_module_base* is the name of the custom Odoo module. In this example, the invoice ODT file is located in *my_custom_module_base/report/account_invoice.odt*. +It's also possible to reference a template located in a trusted path of your +Odoo server. In this case you must let the *module* entry empty and specify +the path to the template as *py3o_template_fallback*. + +.. code:: + + + + + + py3o + odt + /field> + /odoo/templates/py3o/report/account_invoice.odt + + + + +Moreover you must also modify the odoo server configuration file to declare +the allowed root directory for your py3o templates. Only templates located +into this directory can be loaded by py3o report. + +.. code:: + + [options] + ... + + [report_py3o] + root_tmpl_path=/odoo/templates/py3o + If you want an invoice in PDF format instead of ODT format, the XML file should look like: .. code:: diff --git a/report_py3o/models/py3o_report.py b/report_py3o/models/py3o_report.py index 5aa5d574..d8f00e74 100644 --- a/report_py3o/models/py3o_report.py +++ b/report_py3o/models/py3o_report.py @@ -19,7 +19,7 @@ from zipfile import ZipFile, ZIP_DEFLATED from odoo.exceptions import AccessError from odoo.exceptions import UserError from odoo.report.report_sxw import rml_parse -from odoo import api, fields, models, _ +from odoo import api, fields, models, tools, _ logger = logging.getLogger(__name__) @@ -30,7 +30,7 @@ try: except ImportError: logger.debug('Cannot import py3o.template') try: - from py3o.formats import Formats + from py3o.formats import Formats, UnkownFormatException except ImportError: logger.debug('Cannot import py3o.formats') @@ -82,9 +82,46 @@ class Py3oReport(models.TransientModel): required=True ) + @api.multi + def _is_valid_template_path(self, path): + """ Check if the path is a trusted path for py3o templates. + """ + real_path = os.path.realpath(path) + root_path = tools.config.get_misc('report_py3o', 'root_tmpl_path') + if not root_path: + logger.warning( + "You must provide a root template path into odoo.cfg to be " + "able to use py3o template configured with an absolute path " + "%s", real_path) + return False + is_valid = real_path.startswith(root_path + os.path.sep) + if not is_valid: + logger.warning( + "Py3o template path is not valid. %s is not a child of root " + "path %s", real_path, root_path) + return is_valid + + @api.multi + def _is_valid_template_filename(self, filename): + """ Check if the filename can be used as py3o template + """ + if filename and os.path.isfile(filename): + fname, ext = os.path.splitext(filename) + ext = ext.replace('.', '') + try: + fformat = Formats().get_format(ext) + if fformat and fformat.native: + return True + except UnkownFormatException: + logger.warning("Invalid py3o template %s", filename, + exc_info=1) + logger.warning( + '%s is not a valid Py3o template filename', filename) + return False + @api.multi def _get_template_from_path(self, tmpl_name): - """"Return the template from the path to root of the module if specied + """ Return the template from the path to root of the module if specied or an absolute path on your server """ if not tmpl_name: @@ -97,11 +134,9 @@ class Py3oReport(models.TransientModel): "odoo.addons.%s" % report_xml.module, tmpl_name, ) - elif os.path.isabs(tmpl_name): - # It is an absolute path - flbk_filename = os.path.normcase(os.path.normpath(tmpl_name)) - if flbk_filename and os.path.exists(flbk_filename): - # and it exists on the fileystem + elif self._is_valid_template_path(tmpl_name): + flbk_filename = os.path.realpath(tmpl_name) + if self._is_valid_template_filename(flbk_filename): with open(flbk_filename, 'r') as tmpl: return tmpl.read() return None diff --git a/report_py3o/tests/test_report_py3o.py b/report_py3o/tests/test_report_py3o.py index 131cb563..0ddd7541 100644 --- a/report_py3o/tests/test_report_py3o.py +++ b/report_py3o/tests/test_report_py3o.py @@ -6,10 +6,13 @@ from base64 import b64decode import mock import os import pkg_resources +import shutil import tempfile +from contextlib import contextmanager from py3o.formats import Formats +from odoo import tools from odoo.tests.common import TransactionCase from odoo.exceptions import ValidationError @@ -17,13 +20,29 @@ from ..models.py3o_report import TemplateNotFound from base64 import b64encode +@contextmanager +def temporary_copy(path): + filname, ext = os.path.splitext(path) + tmp_filename = tempfile.mktemp(suffix='.' + ext) + try: + shutil.copy2(path, tmp_filename) + yield tmp_filename + finally: + os.unlink(tmp_filename) + + class TestReportPy3o(TransactionCase): + def setUp(self): + super(TestReportPy3o, self).setUp() + self.report = self.env.ref("report_py3o.res_users_report_py3o") + self.py3o_report = self.env['py3o.report'].create({ + 'ir_actions_report_xml_id': self.report.id}) + def test_no_local_fusion_without_fusion_server(self): - report = self.env.ref("report_py3o.res_users_report_py3o") - self.assertTrue(report.py3o_is_local_fusion) + self.assertTrue(self.report.py3o_is_local_fusion) with self.assertRaises(ValidationError) as e: - report.py3o_is_local_fusion = False + self.report.py3o_is_local_fusion = False self.assertEqual( e.exception.name, "Can not use not native format in local fusion. " @@ -49,17 +68,15 @@ class TestReportPy3o(TransactionCase): "Please specify a Fusion Server") def test_required_py3_filetype(self): - report = self.env.ref("report_py3o.res_users_report_py3o") - self.assertEqual(report.report_type, "py3o") + self.assertEqual(self.report.report_type, "py3o") with self.assertRaises(ValidationError) as e: - report.py3o_filetype = False + self.report.py3o_filetype = False self.assertEqual( e.exception.name, "Field 'Output Format' is required for Py3O report") def test_reports(self): py3o_report = self.env['py3o.report'] - report = self.env.ref("report_py3o.res_users_report_py3o") with mock.patch.object( py3o_report.__class__, '_create_single_report') as patched_pdf: result = tempfile.mktemp('.txt') @@ -67,26 +84,26 @@ class TestReportPy3o(TransactionCase): fp.write('dummy') patched_pdf.return_value = result # test the call the the create method inside our custom parser - report.render_report(self.env.user.ids, - report.report_name, - {}) + self.report.render_report(self.env.user.ids, + self.report.report_name, + {}) self.assertEqual(1, patched_pdf.call_count) # generated files no more exists self.assertFalse(os.path.exists(result)) - res = report.render_report( - self.env.user.ids, report.report_name, {}) + res = self.report.render_report( + self.env.user.ids, self.report.report_name, {}) self.assertTrue(res) py3o_server = self.env['py3o.server'].create({"url": "http://dummy"}) # check the call to the fusion server - report.write({"py3o_filetype": "pdf", - "py3o_server_id": py3o_server.id}) + self.report.write({"py3o_filetype": "pdf", + "py3o_server_id": py3o_server.id}) with mock.patch('requests.post') as patched_post: magick_response = mock.MagicMock() magick_response.status_code = 200 patched_post.return_value = magick_response magick_response.iter_content.return_value = "test result" - res = report.render_report( - self.env.user.ids, report.report_name, {}) + res = self.report.render_report( + self.env.user.ids, self.report.report_name, {}) self.assertEqual(('test result', 'pdf'), res) def test_report_post_process(self): @@ -118,31 +135,38 @@ class TestReportPy3o(TransactionCase): self.assertEqual('test result', b64decode(attachements.datas)) def test_report_template_configs(self): - report = self.env.ref("report_py3o.res_users_report_py3o") # the demo template is specified with a relative path in in the module # path - tmpl_name = report.py3o_template_fallback + tmpl_name = self.report.py3o_template_fallback flbk_filename = pkg_resources.resource_filename( - "odoo.addons.%s" % report.module, + "odoo.addons.%s" % self.report.module, tmpl_name) self.assertTrue(os.path.exists(flbk_filename)) - res = report.render_report( - self.env.user.ids, report.report_name, {}) + res = self.report.render_report( + self.env.user.ids, self.report.report_name, {}) self.assertTrue(res) # The generation fails if the tempalte is not found - report.module = False + self.report.module = False with self.assertRaises(TemplateNotFound), self.env.cr.savepoint(): - report.render_report( - self.env.user.ids, report.report_name, {}) + self.report.render_report( + self.env.user.ids, self.report.report_name, {}) - # the template can also be provided as an abspaath - report.py3o_template_fallback = flbk_filename - res = report.render_report( - self.env.user.ids, report.report_name, {}) - self.assertTrue(res) + # the template can also be provided as an abspath if it's root path + # is trusted + self.report.py3o_template_fallback = flbk_filename + with self.assertRaises(TemplateNotFound): + self.report.render_report( + self.env.user.ids, self.report.report_name, {}) + with temporary_copy(flbk_filename) as tmp_filename: + self.report.py3o_template_fallback = tmp_filename + tools.config.misc['report_py3o'] = { + 'root_tmpl_path': os.path.dirname(tmp_filename)} + res = self.report.render_report( + self.env.user.ids, self.report.report_name, {}) + self.assertTrue(res) # the tempalte can also be provided as a binay field - report.py3o_template_fallback = False + self.report.py3o_template_fallback = False with open(flbk_filename) as tmpl_file: tmpl_data = b64encode(tmpl_file.read()) @@ -150,8 +174,40 @@ class TestReportPy3o(TransactionCase): 'name': 'test_template', 'py3o_template_data': tmpl_data, 'filetype': 'odt'}) - report.py3o_template_id = py3o_template - report.py3o_template_fallback = flbk_filename - res = report.render_report( - self.env.user.ids, report.report_name, {}) + self.report.py3o_template_id = py3o_template + self.report.py3o_template_fallback = flbk_filename + res = self.report.render_report( + self.env.user.ids, self.report.report_name, {}) self.assertTrue(res) + + def test_report_template_fallback_validity(self): + tmpl_name = self.report.py3o_template_fallback + flbk_filename = pkg_resources.resource_filename( + "odoo.addons.%s" % self.report.module, + tmpl_name) + # an exising file in a native format is a valid template if it's + self.assertTrue(self.py3o_report._get_template_from_path( + tmpl_name)) + self.report.module = None + # a directory is not a valid template.. + self.assertFalse(self.py3o_report._get_template_from_path('/etc/')) + self.assertFalse(self.py3o_report._get_template_from_path('.')) + # an vaild template outside the root_tmpl_path is not a valid template + # path + # located in trusted directory + self.report.py3o_template_fallback = flbk_filename + self.assertFalse(self.py3o_report._get_template_from_path( + flbk_filename)) + with temporary_copy(flbk_filename) as tmp_filename: + self.assertTrue(self.py3o_report._get_template_from_path( + tmp_filename)) + # check security + self.assertFalse(self.py3o_report._get_template_from_path( + 'rm -rf . & %s' % flbk_filename)) + # a file in a non native LibreOffice format is not a valid template + with tempfile.NamedTemporaryFile(suffix='.toto')as f: + self.assertFalse(self.py3o_report._get_template_from_path( + f.name)) + # non exising files are not valid template + self.assertFalse(self.py3o_report._get_template_from_path( + '/etc/test.odt'))