[FIX] web: properly escape field values when necessary

This rev. removes some html injections issues:
  - in char and text field widgets
  - in phone field widget in mobile
  - in list views (when no widget is instantiated)

6 files changed, 140 insertions, 14 deletions

diff --git a/addons/web/static/src/js/fields/basic_fields.js b/addons/web/static/src/js/fields/basic_fields.js
index cafa59bbd00..6f757e5414e 100644
--- a/addons/web/static/src/js/fields/basic_fields.js
+++ b/addons/web/static/src/js/fields/basic_fields.js
@@ -240,7 +240,7 @@ var InputField = DebouncedField.extend({
      * @private
      */
     _renderReadonly: function () {
-        this.$el.html(this._formatValue(this.value));
+        this.$el.text(this._formatValue(this.value));
     },
 
     //--------------------------------------------------------------------------
@@ -524,6 +524,13 @@ var FieldMonetary = InputField.extend({
         }
     },
     /**
+     * @override
+     * @private
+     */
+    _renderReadonly: function () {
+        this.$el.html(this._formatValue(this.value));
+    },
+    /**
      * Re-gets the currency as its value may have changed.
      * @see FieldMonetary.resetOnAnyFieldChange
      *
@@ -885,8 +892,11 @@ var FieldPhone = FieldEmail.extend({
     _renderReadonly: function () {
         this._super();
         if (this._canCall()) {
+            // Split phone number into two to prevent Skype app from finding it
             var text = this.$el.text();
-            this.$el.html(text.substr(0, text.length/2) + "­" + text.substr(text.length/2)); // To prevent Skype app to find the phone number
+            var part1 = _.escape(text.substr(0, text.length/2));
+            var part2 = _.escape(text.substr(text.length/2));
+            this.$el.html(part1 + "­" + part2);
         } else {
             this.$el.removeClass('o_form_uri');
         }
@@ -1540,7 +1550,7 @@ var FieldPercentPie = AbstractField.extend({
         this.$leftMask.css({transform: leftDeg, msTransform: leftDeg, mozTransform: leftDeg, webkitTransform: leftDeg});
         this.$rightMask.css({transform: rightDeg, msTransform: rightDeg, mozTransform: rightDeg, webkitTransform: rightDeg});
 
-        this.$pieValue.html(Math.round(value) + '%');
+        this.$pieValue.text(Math.round(value) + '%');
     },
 });
 
@@ -1681,9 +1691,9 @@ var FieldProgressBar = AbstractField.extend({
 
         if (!this.write_mode) {
             if (max_value !== 100) {
-                this.$('.o_progressbar_value').html(utils.human_number(value) + " / " + utils.human_number(max_value));
+                this.$('.o_progressbar_value').text(utils.human_number(value) + " / " + utils.human_number(max_value));
             } else {
-                this.$('.o_progressbar_value').html(utils.human_number(value) + "%");
+                this.$('.o_progressbar_value').text(utils.human_number(value) + "%");
             }
         } else if (isNaN(v)) {
             this.$('.o_progressbar_value').val(this.edit_max_value ? max_value : value);
diff --git a/addons/web/static/src/js/fields/field_utils.js b/addons/web/static/src/js/fields/field_utils.js
index ed3014d9bb6..98fafcef7f1 100644
--- a/addons/web/static/src/js/fields/field_utils.js
+++ b/addons/web/static/src/js/fields/field_utils.js
@@ -52,10 +52,18 @@ function formatBoolean(value) {
  * an empty string.
  *
  * @param {string|false} value
+ * @param {Object} [field]
+ *        a description of the field (note: this parameter is ignored)
+ * @param {Object} [options] additional options
+ * @param {boolean} [options.escape=false] if true, escapes the formatted value
  * @returns {string}
  */
-function formatChar(value) {
-    return typeof value === 'string' ? value : '';
+function formatChar(value, field, options) {
+    value = typeof value === 'string' ? value : '';
+    if (options && options.escape) {
+        value = _.escape(value);
+    }
+    return value;
 }
 
 /**
@@ -189,10 +197,18 @@ function formatInteger(value) {
  * case, we assume that it is a record from a BasicModel.
  *
  * @param {Array|Object|false} value
+ * @param {Object} [field]
+ *        a description of the field (note: this parameter is ignored)
+ * @param {Object} [options] additional options
+ * @param {boolean} [options.escape=false] if true, escapes the formatted value
  * @returns {string}
  */
-function formatMany2one(value) {
-    return value && (_.isArray(value) ? value[1] : value.data.display_name) || '';
+function formatMany2one(value, field, options) {
+    value = value && (_.isArray(value) ? value[1] : value.data.display_name) || '';
+    if (options && options.escape) {
+        value = _.escape(value);
+    }
+    return value;
 }
 
 /**
@@ -270,14 +286,27 @@ function formatMonetary(value, field, options) {
     }
 }
 
-function formatSelection(value, field) {
+/**
+ * Returns a string representing the value of the selection.
+ *
+ * @param {string|false} value
+ * @param {Object} [field]
+ *        a description of the field (note: this parameter is ignored)
+ * @param {Object} [options] additional options
+ * @param {boolean} [options.escape=false] if true, escapes the formatted value
+ */
+function formatSelection(value, field, options) {
     if (!value) {
         return '';
     }
     var val = _.find(field.selection, function (option) {
         return option[0] === value;
     });
-    return val[1];
+    value = val[1];
+    if (options && options.escape) {
+        value = _.escape(value);
+    }
+    return value;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
diff --git a/addons/web/static/src/js/fields/relational_fields.js b/addons/web/static/src/js/fields/relational_fields.js
index 10d72a1398d..d65532165e8 100644
--- a/addons/web/static/src/js/fields/relational_fields.js
+++ b/addons/web/static/src/js/fields/relational_fields.js
@@ -1710,7 +1710,7 @@ var FieldSelection = AbstractField.extend({
      * @private
      */
     _renderReadonly: function () {
-        this.$el.empty().html(this._formatValue(this.value));
+        this.$el.empty().text(this._formatValue(this.value));
     },
 
     //--------------------------------------------------------------------------
diff --git a/addons/web/static/src/js/views/list/list_renderer.js b/addons/web/static/src/js/views/list/list_renderer.js
index 21028b99e02..f748ad9dc50 100644
--- a/addons/web/static/src/js/views/list/list_renderer.js
+++ b/addons/web/static/src/js/views/list/list_renderer.js
@@ -201,7 +201,9 @@ var ListRenderer = BasicRenderer.extend({
                 var field = self.state.fields[column.attrs.name];
                 var value = aggregateValues[column.attrs.name].value;
                 var help = aggregateValues[column.attrs.name].help;
-                var formattedValue = field_utils.format[field.type](value, field, {});
+                var formattedValue = field_utils.format[field.type](value, field, {
+                    escape: true,
+                });
                 $cell.addClass('o_list_number').attr('title', help).html(formattedValue);
             }
             return $cell;
@@ -276,7 +278,10 @@ var ListRenderer = BasicRenderer.extend({
         var name = node.attrs.name;
         var field = this.state.fields[name];
         var value = record.data[name];
-        var formattedValue = field_utils.format[field.type](value, field, { data: record.data });
+        var formattedValue = field_utils.format[field.type](value, field, {
+            data: record.data,
+            escape: true,
+        });
         return $td.html(formattedValue);
     },
     /**
diff --git a/addons/web/static/tests/fields/basic_fields_tests.js b/addons/web/static/tests/fields/basic_fields_tests.js
index 7b9fbd061eb..41530f85c4f 100644
--- a/addons/web/static/tests/fields/basic_fields_tests.js
+++ b/addons/web/static/tests/fields/basic_fields_tests.js
@@ -710,6 +710,33 @@ QUnit.module('basic_fields', {
         _t.database.multi_lang = multiLang;
     });
 
+    QUnit.test('char field does not allow html injections', function (assert) {
+        assert.expect(1);
+
+        var form = createView({
+            View: FormView,
+            model: 'partner',
+            data: this.data,
+            arch: '<form string="Partners">' +
+                    '<sheet>' +
+                        '<group>' +
+                            '<field name="foo"/>' +
+                        '</group>' +
+                    '</sheet>' +
+                '</form>',
+            res_id: 1,
+            viewOptions: {
+                mode: 'edit',
+            },
+        });
+
+        form.$('input').val('<script>throw Error();</script>').trigger('input');
+        form.$buttons.find('.o_form_button_save').click();
+        assert.strictEqual(form.$('.o_field_widget').text(), '<script>throw Error();</script>',
+            'the value should have been properly escaped');
+
+        form.destroy();
+    });
 
     QUnit.module('UrlWidget');
 
@@ -2166,6 +2193,42 @@ QUnit.module('basic_fields', {
         list.destroy();
     });
 
+    QUnit.test('phone field does not allow html injections', function (assert) {
+        assert.expect(1);
+
+        var form = createView({
+            View: FormView,
+            model: 'partner',
+            data: this.data,
+            arch:'<form string="Partners">' +
+                    '<sheet>' +
+                        '<group>' +
+                            '<field name="foo" widget="phone"/>' +
+                        '</group>' +
+                    '</sheet>' +
+                '</form>',
+            res_id: 1,
+            viewOptions: {
+                mode: 'edit',
+            },
+            config: {
+                device: {
+                    size_class: 0,
+                    SIZES: { XS: 0, SM: 1, MD: 2, LG: 3 },
+                }
+            },
+        });
+
+        var val = '<script>throw Error();</script><script>throw Error();</script>';
+        form.$('input').val(val).trigger('input');
+
+        // save
+        form.$buttons.find('.o_form_button_save').click();
+        assert.strictEqual(form.$('.o_field_widget').text().split('\u00AD').join(''), val,
+            "value should have been correctly escaped");
+
+        form.destroy();
+    });
 
     QUnit.module('PriorityWidget');
 
diff --git a/addons/web/static/tests/views/list_tests.js b/addons/web/static/tests/views/list_tests.js
index a74192731ec..79bc48dce99 100644
--- a/addons/web/static/tests/views/list_tests.js
+++ b/addons/web/static/tests/views/list_tests.js
@@ -2166,6 +2166,25 @@ QUnit.module('Views', {
 
         list.destroy();
     });
+
+    QUnit.test('field values are escaped', function (assert) {
+        assert.expect(1);
+        var value = '<script>throw Error();</script>';
+
+        this.data.foo.records[0].foo = value;
+
+        var list = createView({
+            View: ListView,
+            model: 'foo',
+            data: this.data,
+            arch: '<tree editable="top"><field name="foo"/></tree>',
+        });
+
+        assert.strictEqual(list.$('.o_data_cell:first').text(), value,
+            "value should have been escaped");
+
+        list.destroy();
+    });
 });
 
 });