> You don't make an app/website secure by deciding on a list of things you need ...

voicereasonish · on March 26, 2014

Just to clarify:

    var t = document.createTextNode(msg);
    content.appendChild(t);

That code sanitises all possible content in msg. I don't need to list out HTML tags, script/style tags, do special case for unicode exploits, etc.

You need to list what variables are "unsafe", but you don't need to list out the ways they might be unsafe. If it's got the potential to be unsafe, assume it's completely unsafe in every conceivable way, and don't use it in any context apart from as an unsafe text string.

The rookie code is something like:

    msg.replace("something I think is unsafe", "something safer");
    content.innerHTML+=msg;

And agreed. InnerHTML should be removed from browsers.

opendais · on March 26, 2014

Ya, but if they built it so msg='<b>msg</b>' that would remove the bold, no?

So it is a bit more complex than that if they want to enable user markup. https://code.google.com/p/pagedown/source/browse/Markdown.Sa... https://code.google.com/p/pagedown/wiki/PageDown

dethstar · on March 26, 2014

I'm not even a front end guy but I'm pretty sure the field they are adding the user message to should handle the style, not the user message.

opendais · on March 26, 2014

If one uses common choices [e.g. Markdown] that isn't how the parsers are designed.

It is [message] -> [parse] -> [sanitize], generally.

voicereasonish · on March 26, 2014

If you want to enable user markup, then build a simple parser, and use that to generate the correct styling you require.

opendais · on March 26, 2014

My point was:

A) It was not as simple as you suggested if there was markup involved in the message.

B) They'd have to use a parser and I linked to a parser that sanitizes that was once used in a pretty big network of sites.

I'm uncertain if you misunderstood or are simply agreeing with me in a tone of writing that makes it sound like you disagree.

ashearer · on March 26, 2014

I think the point was that it's inherently less safe to allow arbitrary markup and then attempt to sanitize it, than to make a full parser that's incapable of generating unsafe HTML at any stage, all other things being equal.

The safety of widely-deployed Markdown + sanitizer libraries is largely thanks to testing at scale and a history of patches for XSS vulnerabilities.

untog · on March 26, 2014

InnerHTML should be removed from browsers.

While it's still the fastest method for changing the page DOM it shouldn't be. When used outside of user inputted scenarios it's fine.

voicereasonish · on March 26, 2014

It's still absolutely ugly code. And in many cases direct DOM manipulation beats it in terms of speed.