diff --git a/packages/app-cli/tests/md_to_html/sanitize_14.html b/packages/app-cli/tests/md_to_html/sanitize_14.html new file mode 100644 index 000000000..dca7c2748 --- /dev/null +++ b/packages/app-cli/tests/md_to_html/sanitize_14.html @@ -0,0 +1 @@ +XSS \ No newline at end of file diff --git a/packages/app-cli/tests/md_to_html/sanitize_14.md b/packages/app-cli/tests/md_to_html/sanitize_14.md new file mode 100644 index 000000000..bb1744e34 --- /dev/null +++ b/packages/app-cli/tests/md_to_html/sanitize_14.md @@ -0,0 +1 @@ +XSS \ No newline at end of file diff --git a/packages/renderer/htmlUtils.ts b/packages/renderer/htmlUtils.ts index 926dca3ce..592b9643d 100644 --- a/packages/renderer/htmlUtils.ts +++ b/packages/renderer/htmlUtils.ts @@ -144,6 +144,11 @@ class HtmlUtils { .replace(/ { - tagStack.push(name.toLowerCase()); + onopentag: (name: string, attrs: Record) => { + // Note: "name" and attribute names are always lowercase even + // when the input is not. So there is no need to call + // "toLowerCase" on them. + + tagStack.push(name); if (disallowedTags.includes(currentTag())) { disallowedTagDepth++; @@ -202,11 +211,25 @@ class HtmlUtils { // regular harmless attribute that starts with "on" will also // be removed. // 0: https://developer.mozilla.org/en-US/docs/Web/Events - for (const name in attrs) { - if (!attrs.hasOwnProperty(name)) continue; - if (name.length <= 2) continue; - if (name.toLowerCase().substr(0, 2) !== 'on') continue; - delete attrs[name]; + for (const attrName in attrs) { + if (!attrs.hasOwnProperty(attrName)) continue; + if (attrName.length <= 2) continue; + if (attrName.substr(0, 2) !== 'on') continue; + delete attrs[attrName]; + } + + if (name === 'a') { + // Make sure that only non-acceptable URLs are filtered out. + // In particular we want to exclude `javascript:` URLs. + if ('href' in attrs && !this.isAcceptedUrl(attrs['href'])) { + attrs['href'] = '#'; + } + + // We need to clear any such attribute, otherwise it will + // make any arbitrary link open within the application. + if ('data-from-md' in attrs) { + delete attrs['data-from-md']; + } } if (options.addNoMdConvClass) { @@ -222,7 +245,7 @@ class HtmlUtils { // attribute. It doesn't always happen and it seems to depend on // what else is in the note but in any case adding the "href" // fixes it. https://github.com/laurent22/joplin/issues/5687 - if (name.toLowerCase() === 'a' && !attrs['href']) { + if (name === 'a' && !attrs['href']) { attrs['href'] = '#'; }