4 ms·

Escape the output based on the context a string is being used in versus trying to sanitize for all use cases on input. This will guarantee that you’re safe no

Escape the output based on the context a string is being used in versus trying to sanitize for all use cases on input. This will guarantee that you’re safe no matter how a piece of content is used tomorrow (just need a new escaping function for that content type), and prevent awkward things like not letting users use “unsafe” strings as input. JSX and XHP are example templating systems that understand context and escape appropriately. If a user wants their title to be “hello%0a%3C%25%3D%20File.open%28%27flag.txt%27%29.read%20%25%3E”, so be it. Use input validation / parsing to ensure data types aren’t violated, but not as an output safety mechanism.

Jerrrry 3y ago
>If a user wants their title to be “hello%0a%3C%25%3D%20File.open%28%27flag.txt%27%29.read%20%25%3E”, so be it. that's a good way to horizontally propagate/reflect XSS and other Code As Data vulnerabilities. better to strip the known-bad/problematic characters https://en.wikipedia.org/wiki/Code_as_data https://en.wikipedia.org/wiki/Code_as_data
- ec109685 3y ago
  The known problematic characters are different in json, xml, css, html content, html attributes, MySQL, etc. Unless you have output escaping, it is hard to ensure everything gets caught, no matter how the data enters the system.
  - tsimionescu 3y ago
    Sure, but there is a common set of safe characters that are guaranteed not to cause problems in any of these: the set described by the regex [a-zA-Z0-9 -]. If you can limit user input to this set, you'll drastically reduce the risk of code injection regardless of the stack below you.
- phyzome 3y ago
  And that's how you end up pissing off users with apostrophes in their names.
  - Terr_ 3y ago
    "Alright. If you’re gonna go ahead with it, I want to make sure you get one thing right. It’s “O’Neill,” with two L’s. There is another Colonel O’Neil with only one L and he has no sense of humor at all."
  - Jerrrry 3y ago
    one apostrophe? sure. more than two in a row? no. Ku' 'Laangah't is valid.
tsimionescu 3y ago
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.

Jerrrry 3y ago

>If a user wants their title to be “hello%0a%3C%25%3D%20File.open%28%27flag.txt%27%29.read%20%25%3E”, so be it. that's a good way to horizontally propagate/reflect XSS and other Code As Data vulnerabilities. better to strip the known-bad/problematic characters https://en.wikipedia.org/wiki/Code_as_data https://en.wikipedia.org/wiki/Code_as_data

ec109685 3y ago
The known problematic characters are different in json, xml, css, html content, html attributes, MySQL, etc. Unless you have output escaping, it is hard to ensure everything gets caught, no matter how the data enters the system.
- tsimionescu 3y ago
  Sure, but there is a common set of safe characters that are guaranteed not to cause problems in any of these: the set described by the regex [a-zA-Z0-9 -]. If you can limit user input to this set, you'll drastically reduce the risk of code injection regardless of the stack below you.
phyzome 3y ago
And that's how you end up pissing off users with apostrophes in their names.
- Terr_ 3y ago
  "Alright. If you’re gonna go ahead with it, I want to make sure you get one thing right. It’s “O’Neill,” with two L’s. There is another Colonel O’Neil with only one L and he has no sense of humor at all."
- Jerrrry 3y ago
  one apostrophe? sure. more than two in a row? no. Ku' 'Laangah't is valid.

tsimionescu 3y ago

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.