Skip to content

Commit

Permalink
SECURITY VULNERABILITY FIX: Fix XSS bug reported today 4 hours ago by…
Browse files Browse the repository at this point in the history 
… 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](#2593)
so features related to that are now removed.
After this fix, Javascript in input fields is not executed.

Thanks to Cyb3rjunky and xet7 !
  • Loading branch information
@xet7
xet7 committed Mar 23, 2020
1 parent 3a6303e commit 482682e
Showing 1 changed file with 68 additions and 147 deletions.
215 changes: 68 additions & 147 deletions client/components/main/editor.js
View file
Original file line number Diff line number Diff line change
@@ -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
{
Expand All @@ -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),
Expand All @@ -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']],
Expand All @@ -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}`,
Expand All @@ -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() {
Expand All @@ -215,6 +138,7 @@ Template.editor.onRendered(() => {
});
}
},

onImageUpload(files) {
const $summernote = getSummernote(this);
if (files && files.length > 0) {
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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
Expand All @@ -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;
Expand All @@ -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();
}
},
});

0 comments on commit 482682e

Please sign in to comment.