From 2b26bbe78a1a2b8b427963a6c44c3853efdb737e Mon Sep 17 00:00:00 2001 From: Lauri Ojansivu Date: Fri, 6 Mar 2020 03:52:12 +0200 Subject: Fix: img tag did not allow width and height. Removed swipebox from markdown editor img tag and updated marked markdown to newest version. Thanks to hradec and xet7 ! Closes #2956 --- client/components/main/editor.js | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'client/components/main/editor.js') diff --git a/client/components/main/editor.js b/client/components/main/editor.js index 39c03aa9..272be197 100755 --- a/client/components/main/editor.js +++ b/client/components/main/editor.js @@ -57,6 +57,9 @@ const sanitizeXss = (input, options) => { } } } + /* 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'); @@ -64,6 +67,7 @@ const sanitizeXss = (input, options) => { return ``; } } + */ } return undefined; }, -- cgit v1.2.3-1-g7c22 From 482682e50079d70c5113169020d6834013b57c11 Mon Sep 17 00:00:00 2001 From: Lauri Ojansivu Date: Mon, 23 Mar 2020 22:29:20 +0200 Subject: 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 ! --- client/components/main/editor.js | 215 +++++++++++++-------------------------- 1 file changed, 68 insertions(+), 147 deletions(-) (limited to 'client/components/main/editor.js') 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 ``; - } else { - // remove 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 ``; - } - } - } - /* 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 ``; - } - } - */ - } - 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(/( { } 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(); - } }, }); -- cgit v1.2.3-1-g7c22 From 12ab8fac5db9c5ac8069d0ca2bca340d6004a25b Mon Sep 17 00:00:00 2001 From: Lauri Ojansivu Date: Tue, 24 Mar 2020 11:04:04 +0200 Subject: Fix Rich editor can not be disabled, regression from changes yesterday at Wekan v3.85. Thanks to uusijani, vjrj and xet7 ! Closes #2967, closes #104 --- client/components/main/editor.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'client/components/main/editor.js') diff --git a/client/components/main/editor.js b/client/components/main/editor.js index 97a96b8e..18b823a2 100755 --- a/client/components/main/editor.js +++ b/client/components/main/editor.js @@ -1,7 +1,5 @@ Template.editor.onRendered(() => { const textareaSelector = 'textarea'; - const enableRicherEditor = - Meteor.settings.public.RICHER_CARD_COMMENT_EDITOR || true; const mentions = [ // User mentions { @@ -32,7 +30,7 @@ Template.editor.onRendered(() => { autosize($textarea); $textarea.escapeableTextComplete(mentions); }; - if (enableRicherEditor) { + if (Meteor.settings.public.RICHER_CARD_COMMENT_EDITOR !== false) { const isSmall = Utils.isMiniScreen(); const toolbar = isSmall ? [ -- cgit v1.2.3-1-g7c22 From b9099a8b7ea6f63c79bdcbb871cb993b2cb7e325 Mon Sep 17 00:00:00 2001 From: Lauri Ojansivu Date: Tue, 24 Mar 2020 20:39:49 +0200 Subject: 1) Fix Pasting text into a card is adding a line before and after (and multiplies by pasting more) by changing paste "p" to "br". 2) Fixes to summernote and markdown comment editors, related to keeping them open when adding comments, having @member mention not close card, and disabling clicking of @member mention. Thanks to xet7 ! Closes #2890 --- client/components/main/editor.js | 51 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 46 insertions(+), 5 deletions(-) (limited to 'client/components/main/editor.js') diff --git a/client/components/main/editor.js b/client/components/main/editor.js index 18b823a2..3f09d284 100755 --- a/client/components/main/editor.js +++ b/client/components/main/editor.js @@ -30,7 +30,7 @@ Template.editor.onRendered(() => { autosize($textarea); $textarea.escapeableTextComplete(mentions); }; - if (Meteor.settings.public.RICHER_CARD_COMMENT_EDITOR !== false) { + if (Meteor.settings.public.RICHER_CARD_COMMENT_EDITOR === 'true') { const isSmall = Utils.isMiniScreen(); const toolbar = isSmall ? [ @@ -108,10 +108,37 @@ Template.editor.onRendered(() => { } return undefined; }; + // Prevent @member mentions on Add Comment input field + // from closing card, part 1. + let popupShown = false; inputs.each(function(idx, input) { mSummernotes[idx] = $(input).summernote({ placeholder, + // Prevent @member mentions on Add Comment input field + // from closing card, part 2. + onKeydown(e) { + if (popupShown) { + e.preventDefault(); + } + }, + onKeyup(e) { + if (popupShown) { + e.preventDefault(); + } + }, callbacks: { + // Prevent @member mentions on Add Comment input field + // from closing card, part 3. + onKeydown(e) { + if (popupShown) { + e.preventDefault(); + } + }, + onKeyup(e) { + if (popupShown) { + e.preventDefault(); + } + }, onInit(object) { const originalInput = this; $(originalInput).on('input', function() { @@ -136,7 +163,6 @@ Template.editor.onRendered(() => { }); } }, - onImageUpload(files) { const $summernote = getSummernote(this); if (files && files.length > 0) { @@ -215,6 +241,12 @@ Template.editor.onRendered(() => { const thisNote = this; const updatePastedText = function(object) { const someNote = getSummernote(object); + // Fix Pasting text into a card is adding a line before and after + // (and multiplies by pasting more) by changing paste "p" to "br". + // Fixes https://github.com/wekan/wekan/2890 . + // == Fix Start == + someNote.execCommand('defaultParagraphSeparator', false, 'br'); + // == Fix End == 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('reset'); //clear original @@ -291,11 +323,17 @@ Blaze.Template.registerHelper( } const linkValue = [' ', at, knowedUser.username]; - let linkClass = 'atMention js-open-member'; + //let linkClass = 'atMention js-open-member'; + let linkClass = 'atMention'; if (knowedUser.userId === Meteor.userId()) { linkClass += ' me'; } - const link = HTML.A( + // This @user mention link generation did open same Wekan + // window in new tab, so now A is changed to U so it's + // underlined and there is no link popup. This way also + // text can be selected more easily. + //const link = HTML.A( + const link = HTML.U( { class: linkClass, // XXX Hack. Since we stringify this render function result below with @@ -329,7 +367,10 @@ Template.viewer.events({ const userId = event.currentTarget.dataset.userid; if (userId) { - Popup.open('member').call({ userId }, event, templateInstance); + // Prevent @member mentions on Add Comment input field + // from closing card, part 4. + PopupNoClose.open('member').call({ userId }, event, templateInstance); + event.preventDefault(); } else { const href = event.currentTarget.href; if (href) { -- cgit v1.2.3-1-g7c22 From 3546d7aa02bc65cf1183cb493adeb543ba51945d Mon Sep 17 00:00:00 2001 From: Lauri Ojansivu Date: Tue, 31 Mar 2020 16:56:32 +0300 Subject: Fix Browser always reload the whole page when I change one of the card color. Fixed by making label colors and text again editable. Regression from [Wekan v3.86 2)](https://github.com/wekan/wekan/commit/b9099a8b7ea6f63c79bdcbb871cb993b2cb7e325). Thanks to javen9881 and xet7 ! Closes #2971 --- client/components/main/editor.js | 38 ++++---------------------------------- 1 file changed, 4 insertions(+), 34 deletions(-) (limited to 'client/components/main/editor.js') diff --git a/client/components/main/editor.js b/client/components/main/editor.js index 3f09d284..86c0078f 100755 --- a/client/components/main/editor.js +++ b/client/components/main/editor.js @@ -30,7 +30,7 @@ Template.editor.onRendered(() => { autosize($textarea); $textarea.escapeableTextComplete(mentions); }; - if (Meteor.settings.public.RICHER_CARD_COMMENT_EDITOR === 'true') { + if (Meteor.settings.public.RICHER_CARD_COMMENT_EDITOR !== false) { const isSmall = Utils.isMiniScreen(); const toolbar = isSmall ? [ @@ -108,37 +108,10 @@ Template.editor.onRendered(() => { } return undefined; }; - // Prevent @member mentions on Add Comment input field - // from closing card, part 1. - let popupShown = false; inputs.each(function(idx, input) { mSummernotes[idx] = $(input).summernote({ placeholder, - // Prevent @member mentions on Add Comment input field - // from closing card, part 2. - onKeydown(e) { - if (popupShown) { - e.preventDefault(); - } - }, - onKeyup(e) { - if (popupShown) { - e.preventDefault(); - } - }, callbacks: { - // Prevent @member mentions on Add Comment input field - // from closing card, part 3. - onKeydown(e) { - if (popupShown) { - e.preventDefault(); - } - }, - onKeyup(e) { - if (popupShown) { - e.preventDefault(); - } - }, onInit(object) { const originalInput = this; $(originalInput).on('input', function() { @@ -163,6 +136,7 @@ Template.editor.onRendered(() => { }); } }, + onImageUpload(files) { const $summernote = getSummernote(this); if (files && files.length > 0) { @@ -323,8 +297,7 @@ Blaze.Template.registerHelper( } const linkValue = [' ', at, knowedUser.username]; - //let linkClass = 'atMention js-open-member'; - let linkClass = 'atMention'; + let linkClass = 'atMention js-open-member'; if (knowedUser.userId === Meteor.userId()) { linkClass += ' me'; } @@ -367,10 +340,7 @@ Template.viewer.events({ const userId = event.currentTarget.dataset.userid; if (userId) { - // Prevent @member mentions on Add Comment input field - // from closing card, part 4. - PopupNoClose.open('member').call({ userId }, event, templateInstance); - event.preventDefault(); + Popup.open('member').call({ userId }, event, templateInstance); } else { const href = event.currentTarget.href; if (href) { -- cgit v1.2.3-1-g7c22 From 033d6710470b2ecd7a0ec0b2f0741ff459e68b32 Mon Sep 17 00:00:00 2001 From: Lauri Ojansivu Date: Tue, 31 Mar 2020 23:17:58 +0300 Subject: Fix richer editor submit did not clear edit area. Thanks to xet7 ! --- client/components/main/editor.js | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) (limited to 'client/components/main/editor.js') diff --git a/client/components/main/editor.js b/client/components/main/editor.js index 86c0078f..081c6521 100755 --- a/client/components/main/editor.js +++ b/client/components/main/editor.js @@ -114,12 +114,11 @@ Template.editor.onRendered(() => { callbacks: { onInit(object) { const originalInput = this; - $(originalInput).on('input', function() { + $(originalInput).on('submitted', 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('reset'); - object && object.editingArea.find('.note-placeholder').show(); + sn && sn.summernote('code', ''); } }); const jEditor = object && object.editable; @@ -223,7 +222,7 @@ Template.editor.onRendered(() => { // == Fix End == 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('reset'); //clear original + someNote.summernote('code', ''); //clear original someNote.summernote('pasteHTML', cleaned); //this sets the displayed content editor to the cleaned pasted code. }; setTimeout(function() { @@ -290,7 +289,8 @@ Blaze.Template.registerHelper( let currentMention; while ((currentMention = mentionRegex.exec(content)) !== null) { - const [fullMention, username] = currentMention; + const [fullMention, quoteduser, simple] = currentMention; + const username = quoteduser || simple; const knowedUser = _.findWhere(knowedUsers, { username }); if (!knowedUser) { continue; @@ -330,14 +330,7 @@ Template.viewer.events({ // 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) { - 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(); - + let prevent = true; const userId = event.currentTarget.dataset.userid; if (userId) { Popup.open('member').call({ userId }, event, templateInstance); @@ -347,5 +340,14 @@ Template.viewer.events({ 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(); + } }, }); -- cgit v1.2.3-1-g7c22