Merge pull request #1730 from PrivateBin/xss/jsImprove

Simplify parsing untrusted code and strengthen DOMPurify config

js/privatebin.js

@@ -61,6 +61,20 @@ jQuery.PrivateBin = (function($) {
         }
     };
 
+    /**
+     * DOMpurify settings for HTML content, where only a strict subset is allowed.
+     *
+     * NOTE: The key {@link purifyHtmlConfig.USE_PROFILES} **must not** be included,
+     * as otherwise `USE_PROFILES` takes precedence over {@link purifyHtmlConfigStrictSubset.ALLOWED_TAGS}.
+     *
+     * @private
+     */
+   const purifyHtmlConfigStrictSubset = {
+        ALLOWED_URI_REGEXP: purifyHtmlConfig.ALLOWED_URI_REGEXP,
+        ALLOWED_TAGS: ['a', 'i', 'span', 'kbd'],
+        ALLOWED_ATTR: ['href', 'id']
+    };
+
     /**
      * DOMpurify settings for SVG content
      *
@@ -439,7 +453,7 @@ jQuery.PrivateBin = (function($) {
                         /(((https?|ftp):\/\/[\w?!=&.\/-;#@~%+*-]+(?![\w\s?!&.\/;#~%"=-]>))|((magnet):[\w?=&.\/-;#@~%+*-]+))/ig,
                         '<a href="$1" rel="nofollow noopener noreferrer">$1</a>'
                     ),
-                    purifyHtmlConfig
+                    purifyHtmlConfigStrictSubset
                 )
             );
         };
@@ -814,12 +828,7 @@ jQuery.PrivateBin = (function($) {
 
             if (containsHtml) {
                 // only allow tags/attributes we actually use in translations
-                output = DOMPurify.sanitize(
+                output = DOMPurify.sanitize(output, purifyHtmlConfigStrictSubset);
-                    output, {
-                        ALLOWED_TAGS: ['a', 'i', 'span', 'kbd'],
-                        ALLOWED_ATTR: ['href', 'id']
-                    }
-                );
             }
 
             // if $element is given, insert translation
@@ -966,13 +975,9 @@ jQuery.PrivateBin = (function($) {
          * @returns {boolean}
          */
         function isStringContainsHtml(messageId) {
-            // An integer which specifies the type of the node. An Element node like <p> or <div>.
+            // message IDs are allowed to contain anchors, spans, keyboard and emphasis tags
-            const elementNodeType = 1;
+            // we can recognize all of them by only checking for anchors and keyboard tags
-
+            return messageId.indexOf('<a') !== -1 || messageId.indexOf('<kbd') !== -1;
-            const div = document.createElement('div');
-            div.innerHTML = messageId;
-
-            return Array.from(div.childNodes).some(node => node.nodeType === elementNodeType);
         }
 
         return me;

js/test/I18n.js

@@ -77,7 +77,8 @@ describe('I18n', function () {
                 postfix   =   postfix.replace(/%(s|d)/g, '%%');
                 const translation = DOMPurify.sanitize(
                     prefix + '<a href="' + params[0] + '"></a>' + postfix, {
-                        ALLOWED_TAGS: ['a', 'i', 'span'],
+                        ALLOWED_URI_REGEXP: /^(?:(?:(?:f|ht)tps?|mailto|magnet):)/i,
+                        ALLOWED_TAGS: ['a', 'i', 'span', 'kbd'],
                         ALLOWED_ATTR: ['href', 'id']
                     }
                 );
@@ -129,7 +130,8 @@ describe('I18n', function () {
                 postfix   =   postfix.replace(/%(s|d)/g, '%%').trim();
                 const translation = DOMPurify.sanitize(
                     prefix + '<a href="' + params[0] + '"></a>' + postfix, {
-                        ALLOWED_TAGS: ['a', 'i', 'span'],
+                        ALLOWED_URI_REGEXP: /^(?:(?:(?:f|ht)tps?|mailto|magnet):)/i,
+                        ALLOWED_TAGS: ['a', 'i', 'span', 'kbd'],
                         ALLOWED_ATTR: ['href', 'id']
                     }
                 );

lib/Configuration.php

@@ -122,7 +122,7 @@ class Configuration
             'js/kjua-0.10.0.js'      => 'sha512-BYj4xggowR7QD150VLSTRlzH62YPfhpIM+b/1EUEr7RQpdWAGKulxWnOvjFx1FUlba4m6ihpNYuQab51H6XlYg==',
             'js/legacy.js'           => 'sha512-RQEo1hxpNc37i+jz/D9/JiAZhG8GFx3+SNxjYnI7jUgirDIqrCSj6QPAAZeaidditcWzsJ3jxfEj5lVm7ZwTRQ==',
             'js/prettify.js'         => 'sha512-puO0Ogy++IoA2Pb9IjSxV1n4+kQkKXYAEUtVzfZpQepyDPyXk8hokiYDS7ybMogYlyyEIwMLpZqVhCkARQWLMg==',
-            'js/privatebin.js'       => 'sha512-n2IW9L2/VnsAJX1gumf7deXcgIqyp1RfnG40Cd8lK+uWNiX7gEqZ+rO6zrAa8hHMNyjbJiqXc/FYSE6xWJmZUw==',
+            'js/privatebin.js'       => 'sha512-6SwOJniNN8RBmAK7yCt4ly2qYyH8OALxB74/K1AJgw+YnZgRCfTDVq1qY1K5Y2QCxCODGGTpAjTqQRExzCqV7g==',
             'js/purify-3.3.0.js'     => 'sha512-lsHD5zxs4lu/NDzaaibe27Vd2t7Cy9JQ3qDHUvDfb4oZvKoWDNEhwUY+4bT3R68cGgpgCYp8U1x2ifeVxqurdQ==',
             'js/showdown-2.1.0.js'   => 'sha512-WYXZgkTR0u/Y9SVIA4nTTOih0kXMEd8RRV6MLFdL6YU8ymhR528NLlYQt1nlJQbYz4EW+ZsS0fx1awhiQJme1Q==',
             'js/zlib-1.3.1-2.js'     => 'sha512-4gT+v+BkBqdVBbKOO4qKGOAzuay+v1FmOLksS+bMgQ08Oo4xEb3X48Xq1Kv2b4HtiCQA7xq9dFRzxal7jmQI7w==',