XSS

[dcposch]

:o

https://stackoverflow.com/a/1732454/432100

i recommend using Preact or similar

it's tiny--only 3kb

if we want to stay with plain JS, i recommend using well-vetted HTML escaping code. Handlebars, for example.

currently, it might be possible for anyone to publish a post that will run arbitrary code in all of their followers' feeds

[eelfroth]

thanks for the suggestion, but @neauoire is pretty adamant about not including anything we haven't written ourselves. so no frameworks or libraries. if this is really a security hole, then we'll have to find some other way to close it. cc @0x0ade

[0x0ade]

We're not using regex to parse HTML tags, but using regex to escape characters. We could as well just use .replace("&", "&amp;").replace("<", "&lt;")...

Handlebars seems to be a complete templating library, way more than we need for HTML escaping. A templating / virtual DOM library would be great to have, but it's a (semi-related but) different issue :)

[0x0ade]

By the way, something I noticed just now is how the & -> & replace could cause some wreckage when using escape_html more than once. We're only using it when "rendering," though, so we shouldn't be affected by that side effect for now.

[aeonofdiscord]

The stackoverflow answer talks about parsing HTML, not escaping it, which is a different problem that's more amenable to regex.

The escaping code here follows the OWASP guidelines on XSS prevention (https://www.owasp.org/index.php/XSS_(Cross_Site_Scripting)_Prevention_Cheat_Sheet), with the exception of the forward slash, which does look like an oversight.

If you have a proof-of-concept for getting XSS into rotonde that would be helpful here.

[eelfroth]

@0x0ade the infinitely recursing substitution you dreaded is actually happening now: someone has accidentally put slashes in their media name (media/content/name.ext instead of name.ext) and now this error is all over the console

[0x0ade]

I wonder if the following is still required now that we're escaping URIs when "rendering:" https://github.com/Rotonde/rotonde-client/blob/e91fe04b7d0b783223415957e323a18cb6296d0c/scripts/operator.js#L106

Also, I wonder what actually causes this. The media source should be escaped using escape_attr, which currently only takes the apostrophes (') into account...

[0x0ade]

@eelfroth #130 fixes the issues on my end - the media in question now displays properly on my portal :)

[eelfroth]

@0x0ade can confirm. it's fixed!

[neauoire]

Fixed