3 ms·

The output is not the problem here, it is the input. And, if you can get away with, accepting a small set of known-safe characters is much safer than accepting

The output is not the problem here, it is the input. And, if you can get away with, accepting a small set of known-safe characters is much safer than accepting any character and hoping it will be properly escaped at every level. When the user hands you a string and you then pass this down to other bits of code, you can't know if it will be used in an SQL query, a regex, in an error message that will be rendered into HTML, etc. Ideally all layers of your code would handle user input with the utmost care, but that is often very hard to achieve. If you take user input and use it in a regex, it's easy to regex-escape it, but it's much harder to remember that now this whole regex is user input and can't be safely used to, say, construct an SQL query. And even if you remember to properly escape it in the SQL query, it may show up in the returned result, and now if you display that result, you need to be careful to escape it before passing it to some HTML engine. But then none of this works if you did intend to have some SQL syntax in the regex, or some HTML snippets in the DB: you'd need to make all of these technologies aware of which parts of the expressions are safe and which are tainted by user input. And this is all just to prevent code injection type attacks. I haven't even discussed more subtle attacks, like using Unicode look-like characters to confuse other users.

ec109685 3y ago
What was actually invalid about that input? Why shouldn’t that html escaped string be shown as is to the user? I guess I would tweak my first comment and say input filtering is not enough. You must do output filtering to truly be safe.
- tsimionescu 3y ago
  I don't get how you use the term "output" here. However you put it, the problem was feeding the user's input to ERB.new(). The most general solution would have been something that accepted any string, but escaped it properly for ERB.new, I think we all agree on that. But that escaping still needs to be done on the input, not the output of ERB.new. If they were able to inject code there, the output doesn't even matter: you've already lost by the time you get a return, the malicious payload has already run, it doesn't matter what its output was.
  - ec109685 3y ago
    My point was that the web page that the researcher compromised takes input from user and creates a neon version of it. It should be just fine to pass in any character to the site, so a regex deny list is the wrong approach.
int_19h 3y ago
> accepting a small set of known-safe characters is much safer than accepting any character and hoping it will be properly escaped at every level It's also how you end up with apps that people can't use because they reject their perfectly valid legal name, address etc.
- tsimionescu 3y ago
  Absolutely, that's why I said "if you can get away with it". There are situations where this is ok - for example, a company user ID. Even for names, it is often perfectly fine to require someone to spell their name with Latin characters. For example, even if your name is officially 鳥山 in Japan, you will have to spell it out as Toriyama when you leave Japan, both in formal and informal settings, on paper just as much as in electronic forms, since no one would be able to understand it otherwise. And similarly, if your name is Smith, in Japan you will often have to spell it (and sometimes even pronounce it) スミス.

ec109685 3y ago

What was actually invalid about that input? Why shouldn’t that html escaped string be shown as is to the user? I guess I would tweak my first comment and say input filtering is not enough. You must do output filtering to truly be safe.

tsimionescu 3y ago
I don't get how you use the term "output" here. However you put it, the problem was feeding the user's input to ERB.new(). The most general solution would have been something that accepted any string, but escaped it properly for ERB.new, I think we all agree on that. But that escaping still needs to be done on the input, not the output of ERB.new. If they were able to inject code there, the output doesn't even matter: you've already lost by the time you get a return, the malicious payload has already run, it doesn't matter what its output was.
- ec109685 3y ago
  My point was that the web page that the researcher compromised takes input from user and creates a neon version of it. It should be just fine to pass in any character to the site, so a regex deny list is the wrong approach.

int_19h 3y ago

> accepting a small set of known-safe characters is much safer than accepting any character and hoping it will be properly escaped at every level It's also how you end up with apps that people can't use because they reject their perfectly valid legal name, address etc.

tsimionescu 3y ago
Absolutely, that's why I said "if you can get away with it". There are situations where this is ok - for example, a company user ID. Even for names, it is often perfectly fine to require someone to spell their name with Latin characters. For example, even if your name is officially 鳥山 in Japan, you will have to spell it out as Toriyama when you leave Japan, both in formal and informal settings, on paper just as much as in electronic forms, since no one would be able to understand it otherwise. And similarly, if your name is Smith, in Japan you will often have to spell it (and sometimes even pronounce it) スミス.