Skip to content

Commit

Permalink
Desktop, Mobile: Security: Disable SVG tag support in editor to preve…
Browse files Browse the repository at this point in the history
…nt XSS
  • Loading branch information
laurent22 committed May 17, 2023
1 parent 8deba24 commit caf6606
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 7 deletions.
2 changes: 1 addition & 1 deletion packages/app-cli/tests/MdToHtml.ts
Expand Up @@ -35,7 +35,7 @@ describe('MdToHtml', () => {
const mdFilePath = `${basePath}/${mdFilename}`;
const htmlPath = `${basePath}/${filename(mdFilePath)}.html`;

// if (mdFilename !== 'sanitize_9.md') continue;
if (mdFilename !== 'sanitize_15.md') continue;

const mdToHtmlOptions: any = {
bodyOnly: true,
Expand Down
1 change: 1 addition & 0 deletions packages/app-cli/tests/md_to_html/sanitize_15.html
@@ -0,0 +1 @@
<use href="data:image/svg+xml,&lt;svg id=&apos;x&apos; xmlns=&apos;http://www.w3.org/2000/svg&apos;&gt;&lt;image href=&apos;asdf&apos; onerror=&apos;top.require(`child_process`).execSync(`calc.exe`)&apos; /&gt;&lt;/svg&gt;#x" class="jop-noMdConv">
1 change: 1 addition & 0 deletions packages/app-cli/tests/md_to_html/sanitize_15.md
@@ -0,0 +1 @@
<svg><use href="data:image/svg+xml,&lt;svg id='x' xmlns='http://www.w3.org/2000/svg'&gt;&lt;image href='asdf' onerror='top.require(`child_process`).execSync(`calc.exe`)' /&gt;&lt;/svg&gt;#x" />
16 changes: 10 additions & 6 deletions packages/renderer/htmlUtils.ts
Expand Up @@ -183,17 +183,21 @@ class HtmlUtils {

// The BASE tag allows changing the base URL from which files are
// loaded, and that can break several plugins, such as Katex (which
// needs to load CSS files using a relative URL). For that reason
// it is disabled. More info:
// https://github.com/laurent22/joplin/issues/3021
// needs to load CSS files using a relative URL). For that reason it is
// disabled. More info: https://github.com/laurent22/joplin/issues/3021
//
// "link" can be used to escape the parser and inject JavaScript.
// Adding "meta" too for the same reason as it shouldn't be used in
// notes anyway.
// "link" can be used to escape the parser and inject JavaScript. Adding
// "meta" too for the same reason as it shouldn't be used in notes
// anyway.
//
// There are too many issues with SVG tags and to handle them properly
// we should parse them separately. Currently we are not so it is better
// to disable them. SVG graphics are still supported via the IMG tag.
const disallowedTags = [
'script', 'iframe', 'frameset', 'frame', 'object', 'base',
'embed', 'link', 'meta', 'noscript', 'button', 'form',
'input', 'select', 'textarea', 'option', 'optgroup',
'svg',
];

const parser = new htmlparser2.Parser({
Expand Down

4 comments on commit caf6606

@antoniopaolini
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello,

SVG graphics are still supported via the IMG tag

what means this?

I have some svg in various notes, to display some pictures.
How can I display it again by using IMG tag? I need to save as resource and import in note?

@darkHarry
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@antoniopaolini use svg as data URIs
#8396 (comment)

@antoniopaolini
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@darkHarry you means encoded in base64?
Otherwise, could you suggest me where I can read something about this?

@darkHarry
Copy link

@darkHarry darkHarry commented on caf6606 Jul 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.