7 ms·

So, essentially he is saying: 1) go ahead and accept the risky thing from your users, and store it right there in your database, but 2) make sure that you remem

So, essentially he is saying: 1) go ahead and accept the risky thing from your users, and store it right there in your database, but 2) make sure that you remember, in every single place in your code where you read that out of the database, to treat it properly, and 3) make sure that every other programmer, now or at any time in the future, remembers to do this also, in any code they write which reads user input out of the database and puts it on the screen. What a bad idea. Don't leave landmines there for other maintainers of the code to step on. Especially because the other maintainer may actually be you, six months or a year from now. Sanitize your inputs. Also, escape your outputs.

BurningFrog 7y ago
One of my most important rules: Whenever you say, think, or imply something starting with "So from now on we just have to remember...", you're really saying "Let's decide this part of the system will always keep breaking!".
swiley 7y ago
Or maybe just treat strings and user input as opaque objects and leave it to the libraries to escape them.
inimino 7y ago
"The risky thing" here is a text string. So yes, you should be able to accept an arbitrary string and store it in your database without shooting yourself in the foot, and the same when you read it out of your database and put it in HTML, or JSON, or CSV, or XML, or ASN.1, or protobufs, or in another database. Handling escaping in HTML or REST API is web dev 101, this shouldn't be controversial.
coding123 7y ago
I like the way react does it: dangerouslySetInnerHTML={{__html: variable}} Not only can you easily see every place in the code that is using this construct, but, the framework provides a warning. I definitely don't think it's bad to sanitize HTML content. but in general MOST text in a web app should be just text, and rendered with whatever HTML it contains. In very few places should the web application give user rich-text (aka HTML) access. In any place that the application does that, sanitization should be used.
Joker_vD 7y ago
> Sanitize your inputs. Sadly, there is no "the sanitization". JSON, SQL, HTML, CSS, and URI (and the future formats not invented yet) all require different escape schemes, so while you can indeed filter out anything that can be interpreted as an escape sequence in any of those formats, that's not something you can always do. Instead, render your data properly (yes, that includes escaping whatever you're outputting).
- tmpz22 7y ago
  I particularly enjoy React's dangerouslySetInnerHTML syntax ``` <div dangerouslySetInnerHTML={__html: '<p>hello world</p>'} /> ``` Like they're BEGGING you to never use it but understand sometimes it may be necessary or at least is needed to avoid worse hacks to achieve the same function. I've taken to using prefixes like DANGEROUS or UNSAFE in various parts of our codebase to better indicate to the user where extra caution is needed.
jonathan_s 7y ago
No, there is nothing to remember. Every decent HTML templating engine these days will handle all free form text as unsafe and escape it automatically. The same for SQL libraries. Input sanizitation doesn't work, because it doesn't know what is dangerous and what is not dangerous. That depends completely on the output domain, and at the point where the inputs are received, the output domain is often unknown. Data can flow through many layers of business logic and then be passed to an SQL query, an HTML templating engine or anything else. If you don't consider database strings to be free form text when constructing HTML, then there's a good chance there will be vulnerabilities anyway, regardless of whether any sanitization has been applied. The article is fine.
- reflectiv 7y ago
  > Input sanizitation doesn't work, because it doesn't know what is dangerous and what is not dangerous. This is only true if you write or use shitty validation rules. You act like its impossible to do it right...it is not.
  - bcrosby95 7y ago
    This has absolutely nothing to do with shitty validation rules. It has to do with what you're outputting it as. If you're outputting it to HTML, commas are fine. If you're outputting it to CSV, commas are bad. And your validation rules suck if you don't allow commas in any text field because it might be output to CSV someday.
  - the8472 7y ago
    Unicode RTL override can be dangerous in filenames (in the sense of confusing humans, not computers). It is necessary to preserve in content management systems dealing with bidi content. Of course you can try to have different validations on your model, but then you need to make sure to know all output domains on the model level instead of doing it when handing over to the view.
DagAgren 7y ago
That simply does not work. You can't sanitise, escape and reproduce correctly all at the same time. Say you run a blog. I post a comment saying "But in this case, B<A!" This is clearly dangerous input! But it is also exactly what I wanted to say. How do you sanitise this? Change < to < in the database? Now you have to remember to NOT escape that again when outputting! And you have to make sure that, say, your text resources in your UI are all also escaped the exact same way, or you have to remember to escape them DIFFERENTLY than user-provided input. Or maybe you "sanitise" by stripping out dangerous characters like "<". Now you have broken my comment. The only strategy that is at all maintainable is to store the comment as received, and to escape on output. Anything else is massively fragile or broken.
- kochthesecond 7y ago
  Thanks for explaining this better than I could.
- dictum 7y ago
  Maybe I'm overengineering, but couldn't you store the sanitized version as the normal value, and also store and make publicly available the original unsanitized value in an ominously and obviously named key (say, dangerouslyUnsanitizedValue) that happens to be easily greppable/lintable?
  - inimino 7y ago
    The idea that one string is more dangerous than another is the problem.
  - asheroth 7y ago
    How would you determine which value to display? It seems to suffer from the same issue where if you display the sanitized value then the comment is still missing necessary characters, but if you use the unsanitized value then your application will be vulnerable to XSS.
  - GuB-42 7y ago
    I think you are overengineering ;) Plain text can contain anything and it shall be treated as such, it is that simple. As for security, don't assume everything in your database came from a trusted source. Maybe there are remains from an old version of your code that didn't sanitize, maybe you improperly used admin tools that bypassed checks.
alanfranz 7y ago
Yes. And he's saying the right thing. You can't sanitize for all your possible usages.
mikey_p 7y ago
Nope, that doesn't work, because you can never know what context your data will be used it so you can't know what's safe or not up front: See also: http://acko.net/blog/safe-string-theory-for-the-web/ http://acko.net/blog/safe-string-theory-for-the-web/
deleted 7y ago
[deleted]
SahAssar 7y ago
Or treat your database as a user and don't blindly accept what it gives you without validating it. At a previous job I implanted a user record that would have given me admin credentials on the next db migration (which happened pretty regularly) because the developer of the migration tool said "Why should I not trust this data, it came from the database". It was "sanitized" by the app for it's intended use, but not for the ETL tool. If you sanitize your inputs you automatically create the assumption that the database is "safe", but you also have to sanitize it for every potential future usecase that the data might be used for, which is not clear when you are writing the sanitization code. Can you foresee every type of use the data will have in the future? can you know every ETL step that will be written 5, 10 years down the line? If not, it's safer to treat the data as untrusted, and if you are going to do that anyway it's a whole lot easier to just not sanitize since you will otherwise deal with double-sanitization or double-escaping.
- rossdavidh 7y ago
  "...what it gives you..." Right there is the flaw. It's not "you", it's "any programmer who ever works with this system, now or any time in the future.
  - SahAssar 7y ago
    Programmers generally agree that user input is untrusted, right? My point is that should just be extended to the database, and even where it isn't, it sorta already is for attacks.
deleted 7y ago
[deleted]
ris 7y ago
> So, essentially he is saying: 1) go ahead and accept the risky thing from your users, and store it right there in your database Yes. It's hard to know what "risky" is when you're taking input. You don't have the context of its use. What if someone is discussing html in a comment and wants to refer to an example? Or the same for SQL. You're going to be using a heuristic to try and "guess" this, and you're forever going to be trading off annoying your users with security, which is never a good place to be in. > 2) make sure that you remember, in every single place in your code where you read that out of the database, to treat it properly (and 3 more of the same) No. You simply do not use unsafe methods of mixing this data with your output. Use any remotely modern markup templating system which has a way of tracking the escaped status of text and auto-escaping it when necessary. Use an ORM or at least a database connector which has inbuilt parameter escaping support. Do not just do regular string formatting with this kind of stuff, and don't hire the sort of people who do. > Don't leave landmines there for other maintainers of the code to step on You don't know what is a landmine until you know the context of its use. It is much easier to secure a perimeter at one point and know that everything one side of it is safe and everything the other side of it is not safe. The only practical place to put this perimeter is the point of use/output/whatever you want to call it. Having several different places where this data gets mangled and lacking clarity of exactly which pieces of data are safe and which are not at any one point is a recipe for disaster. "But we'll also escape everything on output" - this leads to double-escaping and weird bespoke hacks to work around the resulting artefacts, which themselves will likely open up holes. And this is not even touching on the idea of a data field's safeness "living with" the data. "Field xyz is safe, we sanitize it on input" - cool, but we only started sanitizing the input in September 2019, so any fields from before that are unsafe.
Forge36 7y ago
What if your output isn't always HTML? Now you need to know to decode the database AND reencode in the destination format.
palant 7y ago
No, nobody has to remember anything. You use prepared statements for your database, you use autoescaping templates for the output, and it just happens automatically. I wrote about this four years ago: https://palant.de/2016/03/02/why-you-should-go-with-secure-by-default-for-your-web-application/ https://palant.de/2016/03/02/why-you-should-go-with-secure-b... On the other hand, when you "sanitize" input you get immensely complex code. Whenever you get some data in, you have to remember to sanitize it. You have to know in advance how that data is going to be used and what might be problematic there. Worse yet, as your usage of that data changes (or your understanding of the problems), the data sanitization has to change as well - both for existing data in the database and everywhere where this data comes in. I've seen codebases doing this, where it's impossible to tell whether a particular piece of code is a vulnerability without looking up tons of context. That's the minefield for other maintainers. Don't do this.

BurningFrog 7y ago

One of my most important rules: Whenever you say, think, or imply something starting with "So from now on we just have to remember...", you're really saying "Let's decide this part of the system will always keep breaking!".

swiley 7y ago

Or maybe just treat strings and user input as opaque objects and leave it to the libraries to escape them.

inimino 7y ago

"The risky thing" here is a text string. So yes, you should be able to accept an arbitrary string and store it in your database without shooting yourself in the foot, and the same when you read it out of your database and put it in HTML, or JSON, or CSV, or XML, or ASN.1, or protobufs, or in another database. Handling escaping in HTML or REST API is web dev 101, this shouldn't be controversial.

coding123 7y ago

I like the way react does it: dangerouslySetInnerHTML={{__html: variable}} Not only can you easily see every place in the code that is using this construct, but, the framework provides a warning. I definitely don't think it's bad to sanitize HTML content. but in general MOST text in a web app should be just text, and rendered with whatever HTML it contains. In very few places should the web application give user rich-text (aka HTML) access. In any place that the application does that, sanitization should be used.

Joker_vD 7y ago

> Sanitize your inputs. Sadly, there is no "the sanitization". JSON, SQL, HTML, CSS, and URI (and the future formats not invented yet) all require different escape schemes, so while you can indeed filter out anything that can be interpreted as an escape sequence in any of those formats, that's not something you can always do. Instead, render your data properly (yes, that includes escaping whatever you're outputting).

tmpz22 7y ago
I particularly enjoy React's dangerouslySetInnerHTML syntax ``` <div dangerouslySetInnerHTML={__html: '<p>hello world</p>'} /> ``` Like they're BEGGING you to never use it but understand sometimes it may be necessary or at least is needed to avoid worse hacks to achieve the same function. I've taken to using prefixes like DANGEROUS or UNSAFE in various parts of our codebase to better indicate to the user where extra caution is needed.

jonathan_s 7y ago

No, there is nothing to remember. Every decent HTML templating engine these days will handle all free form text as unsafe and escape it automatically. The same for SQL libraries. Input sanizitation doesn't work, because it doesn't know what is dangerous and what is not dangerous. That depends completely on the output domain, and at the point where the inputs are received, the output domain is often unknown. Data can flow through many layers of business logic and then be passed to an SQL query, an HTML templating engine or anything else. If you don't consider database strings to be free form text when constructing HTML, then there's a good chance there will be vulnerabilities anyway, regardless of whether any sanitization has been applied. The article is fine.

reflectiv 7y ago
> Input sanizitation doesn't work, because it doesn't know what is dangerous and what is not dangerous. This is only true if you write or use shitty validation rules. You act like its impossible to do it right...it is not.
- bcrosby95 7y ago
  This has absolutely nothing to do with shitty validation rules. It has to do with what you're outputting it as. If you're outputting it to HTML, commas are fine. If you're outputting it to CSV, commas are bad. And your validation rules suck if you don't allow commas in any text field because it might be output to CSV someday.
- the8472 7y ago
  Unicode RTL override can be dangerous in filenames (in the sense of confusing humans, not computers). It is necessary to preserve in content management systems dealing with bidi content. Of course you can try to have different validations on your model, but then you need to make sure to know all output domains on the model level instead of doing it when handing over to the view.

DagAgren 7y ago

That simply does not work. You can't sanitise, escape and reproduce correctly all at the same time. Say you run a blog. I post a comment saying "But in this case, B<A!" This is clearly dangerous input! But it is also exactly what I wanted to say. How do you sanitise this? Change < to < in the database? Now you have to remember to NOT escape that again when outputting! And you have to make sure that, say, your text resources in your UI are all also escaped the exact same way, or you have to remember to escape them DIFFERENTLY than user-provided input. Or maybe you "sanitise" by stripping out dangerous characters like "<". Now you have broken my comment. The only strategy that is at all maintainable is to store the comment as received, and to escape on output. Anything else is massively fragile or broken.

kochthesecond 7y ago
Thanks for explaining this better than I could.
dictum 7y ago
Maybe I'm overengineering, but couldn't you store the sanitized version as the normal value, and also store and make publicly available the original unsanitized value in an ominously and obviously named key (say, dangerouslyUnsanitizedValue) that happens to be easily greppable/lintable?
- inimino 7y ago
  The idea that one string is more dangerous than another is the problem.
- asheroth 7y ago
  How would you determine which value to display? It seems to suffer from the same issue where if you display the sanitized value then the comment is still missing necessary characters, but if you use the unsanitized value then your application will be vulnerable to XSS.
- GuB-42 7y ago
  I think you are overengineering ;) Plain text can contain anything and it shall be treated as such, it is that simple. As for security, don't assume everything in your database came from a trusted source. Maybe there are remains from an old version of your code that didn't sanitize, maybe you improperly used admin tools that bypassed checks.

alanfranz 7y ago

Yes. And he's saying the right thing. You can't sanitize for all your possible usages.

mikey_p 7y ago

Nope, that doesn't work, because you can never know what context your data will be used it so you can't know what's safe or not up front: See also: http://acko.net/blog/safe-string-theory-for-the-web/ http://acko.net/blog/safe-string-theory-for-the-web/

deleted 7y ago

[deleted]

SahAssar 7y ago

Or treat your database as a user and don't blindly accept what it gives you without validating it. At a previous job I implanted a user record that would have given me admin credentials on the next db migration (which happened pretty regularly) because the developer of the migration tool said "Why should I not trust this data, it came from the database". It was "sanitized" by the app for it's intended use, but not for the ETL tool. If you sanitize your inputs you automatically create the assumption that the database is "safe", but you also have to sanitize it for every potential future usecase that the data might be used for, which is not clear when you are writing the sanitization code. Can you foresee every type of use the data will have in the future? can you know every ETL step that will be written 5, 10 years down the line? If not, it's safer to treat the data as untrusted, and if you are going to do that anyway it's a whole lot easier to just not sanitize since you will otherwise deal with double-sanitization or double-escaping.

rossdavidh 7y ago
"...what it gives you..." Right there is the flaw. It's not "you", it's "any programmer who ever works with this system, now or any time in the future.
- SahAssar 7y ago
  Programmers generally agree that user input is untrusted, right? My point is that should just be extended to the database, and even where it isn't, it sorta already is for attacks.

deleted 7y ago

[deleted]

ris 7y ago

> So, essentially he is saying: 1) go ahead and accept the risky thing from your users, and store it right there in your database Yes. It's hard to know what "risky" is when you're taking input. You don't have the context of its use. What if someone is discussing html in a comment and wants to refer to an example? Or the same for SQL. You're going to be using a heuristic to try and "guess" this, and you're forever going to be trading off annoying your users with security, which is never a good place to be in. > 2) make sure that you remember, in every single place in your code where you read that out of the database, to treat it properly (and 3 more of the same) No. You simply do not use unsafe methods of mixing this data with your output. Use any remotely modern markup templating system which has a way of tracking the escaped status of text and auto-escaping it when necessary. Use an ORM or at least a database connector which has inbuilt parameter escaping support. Do not just do regular string formatting with this kind of stuff, and don't hire the sort of people who do. > Don't leave landmines there for other maintainers of the code to step on You don't know what is a landmine until you know the context of its use. It is much easier to secure a perimeter at one point and know that everything one side of it is safe and everything the other side of it is not safe. The only practical place to put this perimeter is the point of use/output/whatever you want to call it. Having several different places where this data gets mangled and lacking clarity of exactly which pieces of data are safe and which are not at any one point is a recipe for disaster. "But we'll also escape everything on output" - this leads to double-escaping and weird bespoke hacks to work around the resulting artefacts, which themselves will likely open up holes. And this is not even touching on the idea of a data field's safeness "living with" the data. "Field xyz is safe, we sanitize it on input" - cool, but we only started sanitizing the input in September 2019, so any fields from before that are unsafe.

Forge36 7y ago

What if your output isn't always HTML? Now you need to know to decode the database AND reencode in the destination format.

palant 7y ago

No, nobody has to remember anything. You use prepared statements for your database, you use autoescaping templates for the output, and it just happens automatically. I wrote about this four years ago: https://palant.de/2016/03/02/why-you-should-go-with-secure-by-default-for-your-web-application/ https://palant.de/2016/03/02/why-you-should-go-with-secure-b... On the other hand, when you "sanitize" input you get immensely complex code. Whenever you get some data in, you have to remember to sanitize it. You have to know in advance how that data is going to be used and what might be problematic there. Worse yet, as your usage of that data changes (or your understanding of the problems), the data sanitization has to change as well - both for existing data in the database and everywhere where this data comes in. I've seen codebases doing this, where it's impossible to tell whether a particular piece of code is a vulnerability without looking up tons of context. That's the minefield for other maintainers. Don't do this.