From 1be50fdeafcd2b94f7b2f47d6e5bc4b6ebaceadd Mon Sep 17 00:00:00 2001 From: "Pedro M. Baeza" Date: Mon, 27 Aug 2018 23:36:33 +0200 Subject: [PATCH] [ADD] *: support SVG images Introduce official support for SVG files in the framework, including the following parts: 1. When client-side SVG images are uploaded, the content is displayed until you save using data URI scheme according RFC 2397 [1]. This scheme requires to specify content format. Using hardcoded "image/png" works for all images types except SVG. Type-sniffing is done using "magic byte" detection via the first base64 encode byte, so that the proper data URI scheme can be used. This should not cause SVG-related security problems as the file is displayed through `` tag, which does not allow SVG scripting [2]. 2. Make /web/image controller compatible with SVG 3. Add support for SVG files for company logo, which uses a dedicated controller. 4. Resizing of SVG files is a no-op, as it makes little sense for a vector-based format. We also want to avoid micro-alterations to the SVG document (in "natural" viewport parameters) as we would store multiple copies of the files in the filestore. 5. Because SVG files are inherently dangerous, upload of SVG files is restricted to administrators, either by blocking it directly before saving it in the database (binary fields with attachment=False), or by neutering them to text/plain mimetype (for binary fields with attachment=True) 6. Add tests for the SVG upload cases and for the non-admin uploads. [1] https://tools.ietf.org/html/rfc2397 [2] https://www.w3.org/wiki/SVG_Security Closes #26635 --- addons/web/controllers/main.py | 8 ++- .../web/static/src/js/fields/basic_fields.js | 9 ++- .../src/js/views/kanban/kanban_record.js | 9 ++- .../static/src/js/widgets/widgets.js | 1 + .../static/src/js/slides_upload.js | 1 + odoo/addons/base/models/ir_attachment.py | 3 +- odoo/addons/base/models/res_users.py | 2 +- odoo/addons/base/tests/test_mimetypes.py | 14 +++++ odoo/addons/test_new_api/ir.model.access.csv | 1 + odoo/addons/test_new_api/models.py | 9 +++ .../test_new_api/tests/test_new_fields.py | 56 +++++++++++++++++++ odoo/fields.py | 14 ++++- odoo/tools/image.py | 5 +- odoo/tools/mimetypes.py | 10 ++++ 14 files changed, 134 insertions(+), 8 deletions(-) diff --git a/addons/web/controllers/main.py b/addons/web/controllers/main.py index c2027984c42..ed2e9f13062 100644 --- a/addons/web/controllers/main.py +++ b/addons/web/controllers/main.py @@ -37,6 +37,7 @@ import odoo.modules.registry from odoo.api import call_kw, Environment from odoo.modules import get_resource_path from odoo.tools import crop_image, topological_sort, html_escape, pycompat +from odoo.tools.mimetypes import guess_mimetype from odoo.tools.translate import _ from odoo.tools.misc import str2bool, xlwt, file_open from odoo.tools.safe_eval import safe_eval @@ -1218,8 +1219,11 @@ class Binary(http.Controller): if row and row[0]: image_base64 = base64.b64decode(row[0]) image_data = io.BytesIO(image_base64) - imgext = '.' + (imghdr.what(None, h=image_base64) or 'png') - response = http.send_file(image_data, filename=imgname + imgext, mtime=row[1]) + mimetype = guess_mimetype(image_base64, default='image/png') + imgext = '.' + mimetype.split('/')[1] + if imgext == '.svg+xml': + imgext = '.svg' + response = http.send_file(image_data, filename=imgname + imgext, mimetype=mimetype, mtime=row[1]) else: response = http.send_file(placeholder('nologo.png')) except Exception: diff --git a/addons/web/static/src/js/fields/basic_fields.js b/addons/web/static/src/js/fields/basic_fields.js index 0c28f0f4b61..12b7b8f302c 100644 --- a/addons/web/static/src/js/fields/basic_fields.js +++ b/addons/web/static/src/js/fields/basic_fields.js @@ -1471,12 +1471,19 @@ var FieldBinaryImage = AbstractFieldBinary.extend({ }, }), supportedFieldTypes: ['binary'], + file_type_magic_word: { + '/': 'jpg', + 'R': 'gif', + 'i': 'png', + 'P': 'svg+xml', + }, _render: function () { var self = this; var url = this.placeholder; if (this.value) { if (!utils.is_bin_size(this.value)) { - url = 'data:image/png;base64,' + this.value; + // Use magic-word technique for detecting image type + url = 'data:image/' + (this.file_type_magic_word[this.value[0]] || 'png') + ';base64,' + this.value; } else { url = session.url('/web/image', { model: this.model, diff --git a/addons/web/static/src/js/views/kanban/kanban_record.js b/addons/web/static/src/js/views/kanban/kanban_record.js index 9be296e3e31..7013464ea87 100644 --- a/addons/web/static/src/js/views/kanban/kanban_record.js +++ b/addons/web/static/src/js/views/kanban/kanban_record.js @@ -166,6 +166,12 @@ var KanbanRecord = Widget.extend({ var colorID = this._getColorID(variable); return KANBAN_RECORD_COLORS[colorID]; }, + file_type_magic_word: { + '/': 'jpg', + 'R': 'gif', + 'i': 'png', + 'P': 'svg+xml', + }, /** * @private * @param {string} model the name of the model @@ -179,7 +185,8 @@ var KanbanRecord = Widget.extend({ options = options || {}; var url; if (this.record[field] && this.record[field].value && !utils.is_bin_size(this.record[field].value)) { - url = 'data:image/png;base64,' + this.record[field].value; + // Use magic-word technique for detecting image type + url = 'data:image/' + this.file_type_magic_word[this.record[field].value[0]] + ';base64,' + this.record[field].value; } else if (this.record[field] && ! this.record[field].value) { url = "/web/static/src/img/placeholder.png"; } else { diff --git a/addons/web_editor/static/src/js/widgets/widgets.js b/addons/web_editor/static/src/js/widgets/widgets.js index dd0982bf3be..2c06743b361 100644 --- a/addons/web_editor/static/src/js/widgets/widgets.js +++ b/addons/web_editor/static/src/js/widgets/widgets.js @@ -363,6 +363,7 @@ var ImageWidget = MediaWidget.extend({ if (!noRender) { this.$('input.url').val('').trigger('input').trigger('change'); } + // TODO: Expand this for adding SVG var domain = this.domain.concat(['|', ['mimetype', '=', false], ['mimetype', this.options.document ? 'not in' : 'in', ['image/gif', 'image/jpe', 'image/jpeg', 'image/jpg', 'image/gif', 'image/png']]]); if (needle && needle.length) { domain.push('|', ['datas_fname', 'ilike', needle], ['name', 'ilike', needle]); diff --git a/addons/website_slides/static/src/js/slides_upload.js b/addons/website_slides/static/src/js/slides_upload.js index 8aa18dc7381..aa638882930 100644 --- a/addons/website_slides/static/src/js/slides_upload.js +++ b/addons/website_slides/static/src/js/slides_upload.js @@ -298,6 +298,7 @@ var SlideDialog = Widget.extend({ }); return res; }, + // TODO: Remove this part, as now SVG support in image resize tools is included //Python PIL does not support SVG, so converting SVG to PNG svg_to_png: function () { var img = this.$el.find("img#slide-image")[0]; diff --git a/odoo/addons/base/models/ir_attachment.py b/odoo/addons/base/models/ir_attachment.py index 94b88536932..3cca0bb85a7 100644 --- a/odoo/addons/base/models/ir_attachment.py +++ b/odoo/addons/base/models/ir_attachment.py @@ -254,7 +254,8 @@ class IrAttachment(models.Model): def _check_contents(self, values): mimetype = values['mimetype'] = self._compute_mimetype(values) xml_like = 'ht' in mimetype or 'xml' in mimetype # hta, html, xhtml, etc. - force_text = (xml_like and (not self.env.user._is_admin() or + user = self.env.context.get('binary_field_real_user', self.env.user) + force_text = (xml_like and (not user._is_system() or self.env.context.get('attachments_mime_plainxml'))) if force_text: values['mimetype'] = 'text/plain' diff --git a/odoo/addons/base/models/res_users.py b/odoo/addons/base/models/res_users.py index 247fb367334..850c21d6b3f 100644 --- a/odoo/addons/base/models/res_users.py +++ b/odoo/addons/base/models/res_users.py @@ -449,7 +449,7 @@ class Users(models.Model): if values['company_id'] not in self.env.user.company_ids.ids: del values['company_id'] # safe fields only, so we write as super-user to bypass access rights - self = self.sudo() + self = self.sudo().with_context(binary_field_real_user=self.env.user) res = super(Users, self).write(values) if 'company_id' in values: diff --git a/odoo/addons/base/tests/test_mimetypes.py b/odoo/addons/base/tests/test_mimetypes.py index 2f2f1433c3e..3ca506cb744 100644 --- a/odoo/addons/base/tests/test_mimetypes.py +++ b/odoo/addons/base/tests/test_mimetypes.py @@ -16,6 +16,11 @@ AAAAAAAAAAAAA/9oACAEBAAEFAn//xAAUEQEAAAAAAAAAAAAAAAAAAAAA/9oACAEDAQE/AX//xAAUEQE AA/9oACAECAQE/AX//xAAUEAEAAAAAAAAAAAAAAAAAAAAA/9oACAEBAAY/An//xAAUEAEAAAAAAAAAAAAAAAAAAAAA/9oACAEBA AE/IX//2gAMAwEAAgADAAAAEB//xAAUEQEAAAAAAAAAAAAAAAAAAAAA/9oACAEDAQE/EH//xAAUEQEAAAAAAAAAAAAAAAAAAAAA /9oACAECAQE/EH//xAAUEAEAAAAAAAAAAAAAAAAAAAAA/9oACAEBAAE/EH//2Q==""" +SVG = b"""PD94bWwgdmVyc2lvbj0iMS4wIiBlbmNvZGluZz0iaXNvLTg4NTktMSI/Pgo8IURPQ1RZUEUgc3ZnIFBVQkxJQyAiL +S8vVzNDLy9EVEQgU1ZHIDIwMDAxMTAyLy9FTiIKICJodHRwOi8vd3d3LnczLm9yZy9UUi8yMDAwL0NSLVNWRy0yMDAwMTEwMi9E +VEQvc3ZnLTIwMDAxMTAyLmR0ZCI+Cgo8c3ZnIHdpZHRoPSIxMDAlIiBoZWlnaHQ9IjEwMCUiPgogIDxnIHRyYW5zZm9ybT0idHJ +hbnNsYXRlKDUwLDUwKSI+CiAgICA8cmVjdCB4PSIwIiB5PSIwIiB3aWR0aD0iMTUwIiBoZWlnaHQ9IjUwIiBzdHlsZT0iZmlsbD +pyZWQ7IiAvPgogIDwvZz4KCjwvc3ZnPgo=""" @tagged('standard', 'at_install') @@ -57,6 +62,15 @@ class test_guess_mimetype(unittest.TestCase): mimetype = guess_mimetype(content, default='test') self.assertEqual(mimetype, 'image/gif') + def test_mimetype_svg(self): + content = base64.b64decode(SVG) + mimetype = guess_mimetype(content, default='test') + self.assertTrue(mimetype.startswith('image/svg')) + # Tests that whitespace padded SVG are not detected as SVG + mimetype = guess_mimetype(b" " + content, default='test') + self.assertNotIn("svg", mimetype) + + if __name__ == '__main__': unittest.main() diff --git a/odoo/addons/test_new_api/ir.model.access.csv b/odoo/addons/test_new_api/ir.model.access.csv index 46a658c818d..c825fb51c59 100644 --- a/odoo/addons/test_new_api/ir.model.access.csv +++ b/odoo/addons/test_new_api/ir.model.access.csv @@ -21,3 +21,4 @@ access_test_new_api_compute_protected,access_test_new_api_compute_protected,mode access_test_new_api_multi_compute_inverse,access_test_new_api_multi_compute_inverse,model_test_new_api_multi_compute_inverse,,1,1,1,1 access_test_new_api_recursive,access_test_new_api_recursive,model_test_new_api_recursive,,1,1,1,1 access_test_new_api_cascade,access_test_new_api_cascade,model_test_new_api_cascade,,1,1,1,1 +access_test_new_api_binary_svg,access_test_new_api_binary_svg,model_test_new_api_binary_svg,,1,1,1,1 diff --git a/odoo/addons/test_new_api/models.py b/odoo/addons/test_new_api/models.py index bc0a3e45876..043ee8a0b7d 100644 --- a/odoo/addons/test_new_api/models.py +++ b/odoo/addons/test_new_api/models.py @@ -459,3 +459,12 @@ class ComputeCascade(models.Model): def _compute_baz(self): for record in self: record.baz = "<%s>" % (record.bar or "") + + +class BinarySvg(models.Model): + _name = 'test_new_api.binary_svg' + _description = 'Test SVG upload' + + name = fields.Char(required=True) + image_attachment = fields.Binary(attachment=True) + image_wo_attachment = fields.Binary(attachment=False) diff --git a/odoo/addons/test_new_api/tests/test_new_fields.py b/odoo/addons/test_new_api/tests/test_new_fields.py index 8284f4652c6..caa4eedf4bf 100644 --- a/odoo/addons/test_new_api/tests/test_new_fields.py +++ b/odoo/addons/test_new_api/tests/test_new_fields.py @@ -998,6 +998,62 @@ class TestFields(common.TransactionCase): self.assertEqual(count(message), 1) self.assertEqual(count(message1), 0) + def test_90_binary_svg(self): + from odoo.addons.base.tests.test_mimetypes import SVG + # This should work without problems + self.env['test_new_api.binary_svg'].create({ + 'name': 'Test without attachment', + 'image_wo_attachment': SVG, + }) + # And this gives error + with self.assertRaises(UserError): + self.env['test_new_api.binary_svg'].sudo( + self.env.ref('base.user_demo'), + ).create({ + 'name': 'Test without attachment', + 'image_wo_attachment': SVG, + }) + + def test_91_binary_svg_attachment(self): + from odoo.addons.base.tests.test_mimetypes import SVG + # This doesn't neuter SVG with admin + record = self.env['test_new_api.binary_svg'].create({ + 'name': 'Test without attachment', + 'image_attachment': SVG, + }) + attachment = self.env['ir.attachment'].search([ + ('res_model', '=', record._name), + ('res_field', '=', 'image_attachment'), + ('res_id', '=', record.id), + ]) + self.assertEqual(attachment.mimetype, 'image/svg+xml') + # ...but this should be neutered with demo user + record = self.env['test_new_api.binary_svg'].sudo( + self.env.ref('base.user_demo'), + ).create({ + 'name': 'Test without attachment', + 'image_attachment': SVG, + }) + attachment = self.env['ir.attachment'].search([ + ('res_model', '=', record._name), + ('res_field', '=', 'image_attachment'), + ('res_id', '=', record.id), + ]) + self.assertEqual(attachment.mimetype, 'text/plain') + + def test_92_binary_self_avatar_svg(self): + from odoo.addons.base.tests.test_mimetypes import SVG + demo_user = self.env.ref('base.user_demo') + # User demo changes his own avatar + demo_user.sudo(demo_user).image = SVG + # The SVG file should have been neutered + attachment = self.env['ir.attachment'].search([ + ('res_model', '=', demo_user.partner_id._name), + ('res_field', '=', 'image'), + ('res_id', '=', demo_user.partner_id.id), + ]) + self.assertEqual(attachment.mimetype, 'text/plain') + class TestX2many(common.TransactionCase): def test_search_many2many(self): diff --git a/odoo/fields.py b/odoo/fields.py index 8d2f939fd88..a146a1bfa07 100644 --- a/odoo/fields.py +++ b/odoo/fields.py @@ -10,6 +10,7 @@ from functools import partial from operator import attrgetter import itertools import logging +import base64 import pytz @@ -27,6 +28,7 @@ from .tools import float_repr, float_round, frozendict, html_sanitize, human_siz from .tools import DEFAULT_SERVER_DATE_FORMAT as DATE_FORMAT from .tools import DEFAULT_SERVER_DATETIME_FORMAT as DATETIME_FORMAT from .tools.translate import html_translate, _ +from .tools.mimetypes import guess_mimetype DATE_LENGTH = len(date.today().strftime(DATE_FORMAT)) DATETIME_LENGTH = len(datetime.now().strftime(DATETIME_FORMAT)) @@ -1752,6 +1754,14 @@ class Binary(Field): # on purpose - non base64 data must be passed as a 8bit byte strings. if not value: return None + # Detect if the binary content is an SVG for restricting its upload + # only to system users. + if value[:1] == b'P': # Fast detection of first 6 bits of '<' (0x3C) + decoded_value = base64.b64decode(value) + # Full mimetype detection + if (guess_mimetype(decoded_value).startswith('image/svg') and + not record.env.user._is_system()): + raise UserError(_("Only admins can upload SVG files.")) if isinstance(value, bytes): return psycopg2.Binary(value) try: @@ -1793,7 +1803,9 @@ class Binary(Field): # create the attachments that store the values env = record_values[0][0].env with env.norecompute(): - env['ir.attachment'].sudo().create([{ + env['ir.attachment'].sudo().with_context( + binary_field_real_user=env.user, + ).create([{ 'name': self.name, 'res_model': self.model_name, 'res_field': self.name, diff --git a/odoo/tools/image.py b/odoo/tools/image.py index 887ba823b96..24aa56ced17 100644 --- a/odoo/tools/image.py +++ b/odoo/tools/image.py @@ -51,7 +51,10 @@ def image_resize_image(base64_source, size=(1024, 1024), encoding='base64', file """ if not base64_source: return False - if size == (None, None): + # Return unmodified content if no resize or we etect first 6 bits of '<' + # (0x3C) for SVG documents - This will bypass XML files as well, but it's + # harmless for these purposes + if size == (None, None) or base64_source[:1] == b'P': return base64_source image_stream = io.BytesIO(codecs.decode(base64_source, encoding)) image = Image.open(image_stream) diff --git a/odoo/tools/mimetypes.py b/odoo/tools/mimetypes.py index 59421cf8bb6..4350e8c71a5 100644 --- a/odoo/tools/mimetypes.py +++ b/odoo/tools/mimetypes.py @@ -102,6 +102,13 @@ def _check_olecf(data): return 'application/vnd.ms-powerpoint' return False + +def _check_svg(data): + """This simply checks the existence of the opening and ending SVG tags""" + if b'' in data: + return 'image/svg+xml' + + # for "master" formats with many subformats, discriminants is a list of # functions, tried in order and the first non-falsy value returned is the # selected mime type. If all functions return falsy values, the master @@ -115,6 +122,9 @@ _mime_mappings = ( _Entry('image/png', [b'\x89PNG\r\n\x1A\n'], []), _Entry('image/gif', [b'GIF87a', b'GIF89a'], []), _Entry('image/bmp', [b'BM'], []), + _Entry('image/svg+xml', [b'<'], [ + _check_svg, + ]), # OLECF files in general (Word, Excel, PPT, default to word because why not?) _Entry('application/msword', [b'\xD0\xCF\x11\xE0\xA1\xB1\x1A\xE1', b'\x0D\x44\x4F\x43'], [ _check_olecf