SECURITY VULNERABILITY FIX: Fix XSS bug reported today 4 hours ago by Cyb3rjunky.

Logged in users could run javascript in input fields.
This affects Wekan versions v3.12-v3.84.
In [Wekan v3.12](https://github.com/wekan/wekan/blob/master/CHANGELOG.md#v312-2019-08-09-wekan-release)
there was [changes for XSS filter to allow inserting images, videos etc
on comment WYSIWYG editor](https://github.com/wekan/wekan/pull/2593)
so features related to that are now removed.
After this fix, Javascript in input fields is not executed.

Thanks to Cyb3rjunky and xet7 !

1 files changed, 68 insertions, 147 deletions

diff --git a/client/components/main/editor.js b/client/components/main/editor.js
index 272be197..97a96b8e 100755
--- a/client/components/main/editor.js
+++ b/client/components/main/editor.js
@@ -1,93 +1,7 @@
-import _sanitizeXss from 'xss';
-const ASIS = 'asis';
-const sanitizeXss = (input, options) => {
-  const defaultAllowedIframeSrc = /^(https:){0,1}\/\/.*?(youtube|vimeo|dailymotion|youku)/i;
-  const allowedIframeSrcRegex = (function() {
-    let reg = defaultAllowedIframeSrc;
-    const SAFE_IFRAME_SRC_PATTERN =
-      Meteor.settings.public.SAFE_IFRAME_SRC_PATTERN;
-    try {
-      if (SAFE_IFRAME_SRC_PATTERN !== undefined) {
-        reg = new RegExp(SAFE_IFRAME_SRC_PATTERN, 'i');
-      }
-    } catch (e) {
-      /*eslint no-console: ["error", { allow: ["warn", "error"] }] */
-
-      console.error('Wrong pattern specified', SAFE_IFRAM_SRC_PATTERN, e);
-    }
-    return reg;
-  })();
-  const targetWindow = '_blank';
-  const getHtmlDOM = html => {
-    const i = document.createElement('i');
-    i.innerHTML = html;
-    return i.firstChild;
-  };
-  options = {
-    onTag(tag, html, options) {
-      const htmlDOM = getHtmlDOM(html);
-      const getAttr = attr => {
-        return htmlDOM && attr && htmlDOM.getAttribute(attr);
-      };
-      if (tag === 'iframe') {
-        const clipCls = 'note-vide-clip';
-        if (!options.isClosing) {
-          const iframeCls = getAttr('class');
-          let safe = iframeCls.indexOf(clipCls) > -1;
-          const src = getAttr('src');
-          if (allowedIframeSrcRegex.exec(src)) {
-            safe = true;
-          }
-          if (safe)
-            return `<iframe src='${src}' class="${clipCls}" width=100% height=auto allowfullscreen></iframe>`;
-        } else {
-          // remove </iframe> tag
-          return '';
-        }
-      } else if (tag === 'a') {
-        if (!options.isClosing) {
-          if (getAttr(ASIS) === 'true') {
-            // if has a ASIS attribute, don't do anything, it's a member id
-            return html;
-          } else {
-            const href = getAttr('href');
-            if (href.match(/^((http(s){0,1}:){0,1}\/\/|\/)/)) {
-              // a valid url
-              return `<a href=${href} target=${targetWindow}>`;
-            }
-          }
-        }
-        /* Don't use swipebox on markdown, so that img tag can now use width
-       * and height parameters. https://github.com/wekan/wekan/issues/2956
-       * Previously this was added at https://github.com/wekan/wekan/pull/2593
-      } else if (tag === 'img') {
-        if (!options.isClosing) {
-          const src = getAttr('src');
-          if (src) {
-            return `<a href='${src}' class='swipebox'><img src='${src}' class="attachment-image-preview mCS_img_loaded"></a>`;
-          }
-        }
-      */
-      }
-      return undefined;
-    },
-    onTagAttr(tag, name, value) {
-      if (tag === 'img' && name === 'src') {
-        if (value && value.substr(0, 5) === 'data:') {
-          // allow image with dataURI src
-          return `${name}='${value}'`;
-        }
-      } else if (tag === 'a' && name === 'target') {
-        return `${name}='${targetWindow}'`; // always change a href target to a new window
-      }
-      return undefined;
-    },
-    ...options,
-  };
-  return _sanitizeXss(input, options);
-};
 Template.editor.onRendered(() => {
   const textareaSelector = 'textarea';
+  const enableRicherEditor =
+    Meteor.settings.public.RICHER_CARD_COMMENT_EDITOR || true;
   const mentions = [
     // User mentions
     {
@@ -98,13 +12,7 @@ Template.editor.onRendered(() => {
           currentBoard
             .activeMembers()
             .map(member => {
-              const user = Users.findOne(member.userId);
-              if (user._id === Meteor.userId()) {
-                return null;
-              }
-              const value = user.username;
-              const username =
-                value && value.match(/\s+/) ? `"${value}"` : value;
+              const username = Users.findOne(member.userId).username;
               return username.includes(term) ? username : null;
             })
             .filter(Boolean),
@@ -124,16 +32,15 @@ Template.editor.onRendered(() => {
     autosize($textarea);
     $textarea.escapeableTextComplete(mentions);
   };
-  if (Meteor.settings.public.RICHER_CARD_COMMENT_EDITOR !== false) {
+  if (enableRicherEditor) {
     const isSmall = Utils.isMiniScreen();
     const toolbar = isSmall
       ? [
           ['view', ['fullscreen']],
           ['table', ['table']],
-          ['font', ['bold']],
-          ['color', ['color']],
-          ['insert', ['video']], // iframe tag will be sanitized TODO if iframe[class=note-video-clip] can be added into safe list, insert video can be enabled
+          ['font', ['bold', 'underline']],
           //['fontsize', ['fontsize']],
+          ['color', ['color']],
         ]
       : [
           ['style', ['style']],
@@ -143,11 +50,47 @@ Template.editor.onRendered(() => {
           ['color', ['color']],
           ['para', ['ul', 'ol', 'paragraph']],
           ['table', ['table']],
-          ['insert', ['link', 'picture', 'video']], // iframe tag will be sanitized TODO if iframe[class=note-video-clip] can be added into safe list, insert video can be enabled
+          //['insert', ['link', 'picture', 'video']], // iframe tag will be sanitized TODO if iframe[class=note-video-clip] can be added into safe list, insert video can be enabled
           //['insert', ['link', 'picture']], // modal popup has issue somehow :(
           ['view', ['fullscreen', 'help']],
         ];
-    const cleanPastedHTML = sanitizeXss;
+    const cleanPastedHTML = function(input) {
+      const badTags = [
+        'style',
+        'script',
+        'applet',
+        'embed',
+        'noframes',
+        'noscript',
+        'meta',
+        'link',
+        'button',
+        'form',
+      ].join('|');
+      const badPatterns = new RegExp(
+        `(?:${[
+          `<(${badTags})s*[^>][\\s\\S]*?<\\/\\1>`,
+          `<(${badTags})[^>]*?\\/>`,
+        ].join('|')})`,
+        'gi',
+      );
+      let output = input;
+      // remove bad Tags
+      output = output.replace(badPatterns, '');
+      // remove attributes ' style="..."'
+      const badAttributes = new RegExp(
+        `(?:${[
+          'on\\S+=([\'"]?).*?\\1',
+          'href=([\'"]?)javascript:.*?\\2',
+          'style=([\'"]?).*?\\3',
+          'target=\\S+',
+        ].join('|')})`,
+        'gi',
+      );
+      output = output.replace(badAttributes, '');
+      output = output.replace(/(<a )/gi, '$1target=_ '); // always to new target
+      return output;
+    };
     const editor = '.editor';
     const selectors = [
       `.js-new-comment-form ${editor}`,
@@ -167,45 +110,25 @@ Template.editor.onRendered(() => {
         }
         return undefined;
       };
-      let popupShown = false;
       inputs.each(function(idx, input) {
         mSummernotes[idx] = $(input).summernote({
           placeholder,
           callbacks: {
-            onKeydown(e) {
-              if (popupShown) {
-                e.preventDefault();
-              }
-            },
-            onKeyup(e) {
-              if (popupShown) {
-                e.preventDefault();
-              }
-            },
             onInit(object) {
               const originalInput = this;
-              const setAutocomplete = function(jEditor) {
-                if (jEditor !== undefined) {
-                  jEditor.escapeableTextComplete(mentions).on({
-                    'textComplete:show'() {
-                      popupShown = true;
-                    },
-                    'textComplete:hide'() {
-                      popupShown = false;
-                    },
-                  });
-                }
-              };
-              $(originalInput).on('submitted', function() {
-                // resetCommentInput has been called
+              $(originalInput).on('input', function() {
+                // when comment is submitted, the original textarea will be set to '', so shall we
                 if (!this.value) {
                   const sn = getSummernote(this);
-                  sn && sn.summernote('code', '');
+                  sn && sn.summernote('reset');
+                  object && object.editingArea.find('.note-placeholder').show();
                 }
               });
               const jEditor = object && object.editable;
               const toolbar = object && object.toolbar;
-              setAutocomplete(jEditor);
+              if (jEditor !== undefined) {
+                jEditor.escapeableTextComplete(mentions);
+              }
               if (toolbar !== undefined) {
                 const fBtn = toolbar.find('.btn-fullscreen');
                 fBtn.on('click', function() {
@@ -215,6 +138,7 @@ Template.editor.onRendered(() => {
                 });
               }
             },
+
             onImageUpload(files) {
               const $summernote = getSummernote(this);
               if (files && files.length > 0) {
@@ -295,7 +219,7 @@ Template.editor.onRendered(() => {
                 const someNote = getSummernote(object);
                 const original = someNote.summernote('code');
                 const cleaned = cleanPastedHTML(original); //this is where to call whatever clean function you want. I have mine in a different file, called CleanPastedHTML.
-                someNote.summernote('code', ''); //clear original
+                someNote.summernote('reset'); //clear original
                 someNote.summernote('pasteHTML', cleaned); //this sets the displayed content editor to the cleaned pasted code.
               };
               setTimeout(function() {
@@ -335,6 +259,8 @@ Template.editor.onRendered(() => {
   }
 });
 
+import sanitizeXss from 'xss';
+
 // XXX I believe we should compute a HTML rendered field on the server that
 // would handle markdown and user mentions. We can simply have two
 // fields, one source, and one compiled version (in HTML) and send only the
@@ -356,12 +282,11 @@ Blaze.Template.registerHelper(
       }
       return member;
     });
-    const mentionRegex = /\B@(?:(?:"([\w.\s]*)")|([\w.]+))/gi; // including space in username
+    const mentionRegex = /\B@([\w.]*)/gi;
 
     let currentMention;
     while ((currentMention = mentionRegex.exec(content)) !== null) {
-      const [fullMention, quoteduser, simple] = currentMention;
-      const username = quoteduser || simple;
+      const [fullMention, username] = currentMention;
       const knowedUser = _.findWhere(knowedUsers, { username });
       if (!knowedUser) {
         continue;
@@ -380,42 +305,38 @@ Blaze.Template.registerHelper(
           // `userId` to the popup as usual, and we need to store it in the DOM
           // using a data attribute.
           'data-userId': knowedUser.userId,
-          [ASIS]: 'true',
         },
         linkValue,
       );
 
       content = content.replace(fullMention, Blaze.toHTML(link));
     }
+
     return HTML.Raw(sanitizeXss(content));
   }),
 );
+
 Template.viewer.events({
   // Viewer sometimes have click-able wrapper around them (for instance to edit
   // the corresponding text). Clicking a link shouldn't fire these actions, stop
   // we stop these event at the viewer component level.
   'click a'(event, templateInstance) {
-    let prevent = true;
+    event.stopPropagation();
+
+    // XXX We hijack the build-in browser action because we currently don't have
+    // `_blank` attributes in viewer links, and the transformer function is
+    // handled by a third party package that we can't configure easily. Fix that
+    // by using directly `_blank` attribute in the rendered HTML.
+    event.preventDefault();
+
     const userId = event.currentTarget.dataset.userid;
     if (userId) {
       Popup.open('member').call({ userId }, event, templateInstance);
     } else {
       const href = event.currentTarget.href;
-      const child = event.currentTarget.firstElementChild;
-      if (child && child.tagName === 'IMG') {
-        prevent = false;
-      } else if (href) {
+      if (href) {
         window.open(href, '_blank');
       }
     }
-    if (prevent) {
-      event.stopPropagation();
-
-      // XXX We hijack the build-in browser action because we currently don't have
-      // `_blank` attributes in viewer links, and the transformer function is
-      // handled by a third party package that we can't configure easily. Fix that
-      // by using directly `_blank` attribute in the rendered HTML.
-      event.preventDefault();
-    }
   },
 });