Raneto icon indicating copy to clipboard operation
Raneto copied to clipboard

Sanitize only potential XSS HTML code, rather than all HTML on submit of edit page

Open Syndamia opened this issue 3 years ago • 4 comments

Problem

On the Edit page of any document, on submit, content sanitization is done like this:

https://github.com/ryanlelek/Raneto/blob/2d7e1a21b592a6df03c7d71f217f6fba216f3f58/app/routes/page.edit.route.js#L51

This seems to have been done to fix cross-site scripting issues.

However, I feel it is a bit too aggressive. There are genuine use cases for inline HTML. Definition lists and table colspan (for example) are useful features of HTML that Markdown doesn't support.

Also, backticks are escaped, which prevents code blocks from rendering (blockquotes too are escaped, but they seem to have no styling).

Finally, given that meta tags are stored in the markdown files themselves, meta data also has issues. For example, quotes in description or title are escaped, while they shouldn't!

Proposition

Use a proper cross-site scripting sanitization library, like DOMPurify. It should be able to replace the validator library in page.edit.route.js (and sanitize.js?).

Syndamia avatar Sep 15 '22 17:09 Syndamia

You're right and that's a great suggestion.

The patch #370 was put out quickly #381 is also related

Marking as a bug. I have limited free time at the moment, but will make some time on the weekends. PRs also welcomed if you have the time and interest.

#379 also mentions Blockquotes for reference

ryanlelek avatar Oct 26 '22 04:10 ryanlelek

@Syndamia please check the latest main branch or newest NPM package v0.17.7 (should be published in 24 hours) I loosened the aggressive sanitization because while security is good it's massively affecting the usability like you mentioned.

Related commit: https://github.com/ryanlelek/Raneto/commit/863aaf5095010e1013715e16e4fd474166c2591a

Opinion: Security good, but many users are not (hopefully) letting any public visitor edit. Only trusted members / employees / etc

ryanlelek avatar Feb 21 '24 12:02 ryanlelek

Hello, I'm sorry for the very late response.

Inline HTML

All inline HTML still seems to get escaped. Definition lists seem to be completely unsupported (before I think they were properly parsed and rendered while editing, but then didn't show up on a saved page).

Backticks & quoteblocks

Backticks and quoteblocks seem fixed.

Quotes in metadata

Quotes in metadata still cause issues. Quotes in Title will be saved as-is, while description will be surrounded with single quotes:

---
Title: Test "Number 4"
Description: '"Something important"'
---

This seems to cause issues for the UI, it looks as if any (single or double) quoted text in a metadata block is disregarded. The metadata fields of the aforementioned Markdown looks like this when editing:

image

P.S. On version 0.17.8 all pages disappear from the left "menu", even when I add a new page. No, they're not just hidden by the styling, the HTML for them doesn't even exist. For this reason, all testing was done on 0.17.7

image image

Syndamia avatar Mar 23 '24 06:03 Syndamia

Thank you for the detailed update! I'll look further into this. Seems like the foundation needs a rework, it has been well over 10 years and can use some attention.

ryanlelek avatar Mar 23 '24 23:03 ryanlelek