Hacker News new | past | comments | ask | show | jobs | submit login

This should be a major red flag to anyone.

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

You sanitise everything to start with.

A very common rookie error.




I, for one, am glad to see example Elixir apps with some polish that are published freely. I've been meaning to get into Elixir and Erlang, but lack of polished example apps has been a stumbling block for me, and though I have no immediate need for a TeamChat app at all, it's one of those examples like "The Todos App" that you can even perform as a code-kata in your language of choice.

It would be great if I didn't have to use any Off-the-Shelf code at all, or if I must, if I actually had the time and knowledge to review it for serious vulnerabilities. But posts like this are why I come to HN.


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

I agree

> You sanitise everything to start with.

So you need to list everything you need to sanitise...

A better approach is to ban "innerHTML" from your code. You should always display user generated text in text nodes.


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.


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


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.


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

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


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


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.


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.


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.


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




Consider applying for YC's W25 batch! Applications are open till Nov 12.

Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: