Merge pull request #109 from acsone/10.0-avoid_injections_report_py3o-lmi

[10.0] [IMP] report_py3o: prevent injections when retrieving the template fr…

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::
+
+  <?xml version="1.0" encoding="utf-8"?>
+  <odoo>
+
+  <record id="account.account_invoices" model="ir.actions.report.xml">
+      <field name="report_type">py3o</field>
+      <field name="py3o_filetype">odt</field>
+      <field name="module">/field>
+      <field name="py3o_template_fallback">/odoo/templates/py3o/report/account_invoice.odt</field>
+  </record>
+
+  </odoo>
+
+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::

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

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'))