For example some people think that stripping ZWNJ, ZWJ or other kinds of spaces is a thing they ought to do because it confuses their markup parsers or can be used to encode hidden information in posts or stuff like that. Guess what, it breaks emoji, arabic, some asian languages and a bunch of other things.
If you only sanitize your output and realize you made a mistake you can easily fix it by changing your output sanitzing algorithm. If you santizied your input you threw away data and can't fix your problem.
Output is a different matter though but that’s because of rendering content safely down to HTML, JavaScript or JSON (to name a few examples). SQL shouldn’t come into the equation by this point.
SQL injection is really a problem of how you pass parameters to your SQL layer. Parametrized queries are the (easy and widely available) solution. If you are concatenating input to your SQL queries, you're doing it wrong.
It's crazy.
Even if you're using 1990s technology without parameterised queries, it's not like it's impossible to say `insert into users (name, motto) values ('O\'Brien', 'foo\' OR 1=1;')`.
I wish the controls on browsers came with a green V that implements best practices (8+ symbols, no filter) so that people who made websites understand that this is what they should conform to. Not their own misconceptions about password security.
And yes, in 90s php's security sucked, but that was nothing php specific, it was just the sentiment of that time. Everyone did it, in all languages. I remember using tons of $dbh->do() in Perl's DBI back then, intentionally avoiding to prepare statements for a quick and dirty stuff (and most of the scripts back then were quick and dirty stuff). It's in a big part because we were used to building desktop apps and thinking in terms of security that applied for them like being careful about your pointers and input strings lengths and stack overflows and stuff. Web was still pretty new thing.
Ex-shared hosting bod here, who had the joy of managing our PHP environments :(
Sadly in the real world, even after the great big (and pointless) act of deprecating and removing the mysql_* library, naive developers (and experienced ones that should've know better) just moved onto mysqli_* or PDO and still used string concatenation with raw inputs, instead of learning how to parameterise their queries.
Used to drive me flippin' nuts.
True, I stand corrected, I've just checked and Wordpress still does it just like that: https://github.com/WordPress/WordPress/blob/master/wp-includ...
Anyway, there’s a patch release the next day, and somewhere I find the diff. Now I can’t read PHP but I know what string concatenation looks like, especially if someone does a diff on it. I’ll be damned if the diff didn’t fix one SQL string concatenation that was less than five lines from code with the same structure. Scsry.
"I was the first (paid) employee of reddit" https://www.jedberg.net/
I mean...
Also the methods don’t really change across different SQL databases, at least not conceptually. Sure the RDBMS drivers might change but these days that stuff is usually abstracted away into a single framework for SQL. The real significant change would be switching to a NoSQL database but if you’re doing that then it’s not SQL you need to be “sanitising” anyway.
That's a fair criticism. But that is what I've called it for a long time.
> The real significant change would be switching to a NoSQL database but if you’re doing that then it’s not SQL you need to be “sanitising” anyway.
Right, that's why I started off by saying "Sanitize your inputs so that it can be safely stored in your data store", so that it could apply to any data store.
It's just a terminology argument at this point. But my main point was that you need to still think carefully about how you're going to store your data and do it safely, and then also make things safe on the way out, like the article suggests.
I think many people would agree that "sanitization" is loosely defined and I think that's exactly what led to the misunderstandings that the OP's article is trying to address.
> that's why I started off by saying "Sanitize your inputs so that it can be safely stored in your data store"
That could be interpreted like "escape quotes at the start of the request if you know that you're using a database where quotes have a special meaning" a la PHP magic quotes, which I'm guessing is not what you meant but it is what the OP is criticizing. The key is that the sanitization (or whatever you want to call it) shouldn't happen until you're ready to insert into the DB, otherwise that data will be coupled with database logic through the whole flow of your app
Sanitizing your input means changing them permanently. You don't actually want to do that. You want to store exactly what the original value was, but you want to do it safely. When you retrieve it again, it should be the same as it was originally.
If you "sanitize" the input, you won't necessarily have the original value ever again.
(I've seen the line of thought taken one step further: taking the realization that it's impractical to make strings universally safe for any context—even if you HTML entity-encode it twice, what if a recipient decodes it three times?—and concluding that security is hard and we can only approach it asymptotically, so shrugs XSS-like bugs are normal and unavoidable given finite time & budget.)
If the mindset is more like converting units, it becomes clearer. You can't concatenate HTML with a general Unicode string without converting the string to HTML first, any more than you can add inches and centimeters directly. "Cleaning" the centimeters would make no sense.
I think this is true, but doesn't quite reach the point GP is making: speaking correctly is also important. Calling parameterization input sanitization communicates the wrong message. And abstracting the wrong solution to apply it to a different problem isn't all that helpful. You could just as easily encode or hash input to fit the underlying data format without losing data (except in the case of truncation), but that isn't input sanitization, either.
Input sanitization is strictly checking front-end input against a ruleset and rejecting anything that does not comply. This is fundamentally different than dealing with anything thrown at you and handling it gracefully.
That's validation. Sanitization involves altering the data to make it safe (comes from the word "sanitary").
I think most devs think of sanitization as "make X safe", not "see if X is safe, if not reject" since that is usually called validation.
Using hand sanitizer does not remove your hands if they have harmful bacteria. The hands are still there, just cleaned from (some) of the harmful parts.
> And of course use your SQL engine’s parameterized query features so it properly escapes variables when building SQL:
The spread of natural language processing into systems and analysis tools might increase the scope for this sort of thing.
For example what if your NLP is bootstrapped from a shell script and your database content has been stripped of SQL but still contains stuff that might be interpreted as $(sub-shells)? Before long you run into a situation where literally no characters are considered safe (eg even alpha characters in the English alphabet are used as tokens in some programming languages and "what if someone builds a script in one of those languages?").
The only sane way to address the unknown is to treat raw strings as "dirty" and follow best practices when handling them (plus all the usual processes to properly test your code before it's used in production). In which case you're back to no longer needing input sanitisation.
Many data types have some concept of well-formedness, and in those cases, there are pragmatic reasons for only accepting well-formed input that go even beyond the security aspect.
This is why I got confused when you said "but we should acknowledge..." (ie thinking you were raising a point other than what myself and others had already acknowledged).
> Output is a different matter though
"Output" doesn't have to be front end rendered nor even internet facing. It could also be input for another internal process.
If you're saying people should be aware that handling data safely requires more steps than just parametrised SQL then yes I touched on that, as have others, and it's not something anyone is unaware of. Hence why there has been so many high quality posts discussing the different methods of validation, sanitisation and escaping. So it's a rather strange position to assume when you say "we need to acknowledge" given that's what everyone (including me) has been doing. But it never hurts to be categoric about important points like that so your original post is still relevant.
If you're making some other point then I've already had two stabs at deciphering it and failed both times. So it's really not clear what that point is.
However if your intention was just trolling me then fine, I bit and you won.
That's not sanitizing input, that's escaping output - if you subdivide concerns appropriately. The data you're saving into database is the output of your program.
Really, the problem is of language translation. User input is an unstructured blob. SQL, or HTML, are structured languages, with their own semantics. Whenever you cross the language barrier, you need to translate data from one language to the other. Parametrized queries are the usual API to SQL drivers, and they do this for you under the hood, producing a valid SQL query string[0]. When going to HTML+JS, you need to invoke some library (or do translation yourself).
(I really don't like the term "escaping". Translating between languages is more than just sticking slashes in front of double quotes.)
This is why "sanitizing inputs" is a nonsense concept. The problem is of language translation, and you can't translate if you don't know the destination language. A blob correctly sanitized for SQL will not be correctly sanitized for HTML, and an input correctly sanitized for both will look bad in either.
SQL injection and XSS are the same bug. Failure to translate between languages. Usually caused by a pretty stupid but somehow very popular idea - building target language expressions by gluing plain strings together[1].
--
[0] - Sometimes. I remember this is how it worked in the past, but not sure if server RDBMS APIs haven't changed since. For comparison, with SQLite, you're passing the query with placeholders to the SQLite functions, and the parameter values are passed as arguments. The SQLite internals turn this into an executable query, but I'm pretty sure this does not involve a query string with actual parameter values in it ever existing in memory.
[1] - A related source of footguns is using templating engines for web pages. HTML is a tree of nodes, not a plain string. Using a template system is a recipe for XSS problems.
However, a lot of client side libraries are cheating and embed the parameters into the query string before passing it to the server instead of implementing the proper parts of the protocol. This is about as safe as doing plain string concatenation in the first place. I don't trust the library authors to actually get this right.
For example, what if someone posts a legitimate comment on reddit helping someone with SQL syntax for deleting tables and includes "DROP TABLE users" -- did you sanitize that away?
No, see my edit above. It was just parameterized.
The article makes sense to me; if I’m just storing strings in a database, I don’t see why they would need to be sanitized at rest, even if they contain malicious SQL code. Only when I actually come to use those strings for some purpose.
When you output arbitrary data, you need to encode it in a way that is suitable for that context. These contexts might be:
- Generating a web page.
- Including in a JSON response from an API.
- Sending an email.
- Storing in an SQL database.
These all use different formats / protocols that use different syntax to encode data. How you correctly encode data for one of them is different to how you correctly encode data for another of them. There is no method of taking untrusted data and "sanitising" it so that it is correct for all of them. What works for one will break for the rest.
If you want to handle arbitrary data correctly and safely, store it as-is and when the time comes to use it, encode it appropriately for the context you are using it in. Where possible, use tools and systems that get it right by default instead of requiring developers to remember to encode correctly, e.g. generate HTML with templating engines that encode data as HTML by default, and use parameterised queries with SQL.
Don't, unless you're sure the templating engine actually parses the HTML into a tree of nodes before interpolating and re-emitting it. Otherwise it's likely someone will interpolate something in an improper context, e.g. inside <script> or <style> block.
That still doesn't mean you should care about output sanitization on data entry, as you don't know how it will be output yet.
NO!
If you get bad input, you should fail, loudly. Anything else is a recipe for disaster.
(Not as in "crash the whole system", of course, but as in "reject the request".)
> "sanitizing input" is plain nonsense
> "Unsafe input" is not a thing.
Have you ever used hand sanitizer? The point of hand sanitizer is to reject infectious diseases outright.... you know, so they don’t get IN your body. You seem to have adopted a narrowly defined sense of sanitization which does not include “mercilessly discard/destroy”.
There is no such thing as "infectious input data".
> You seem to have adopted a narrowly defined sense of sanitization which does not include “mercilessly discard/destroy”.
None of the dictionaries I just checked support such a definition. They are all about "changing something to be more sane/sanitary/pleasant/acceptable/...".
Also, mind you a hand sanitizer doesn't destroy your hand, it destroys microbes, in order to make your hand sanitary, so as to enable you to continue using your hand instead of rejecting/disarding it. Which is exactly the kind of thing you should not ever do with input data.
The idea that "accepting all the inputs" somehow gives you an advantage is an illusion: If the semantics of some input are not well-defined, then the only thing you gain by accepting it anyway are hard to debug interoperability problems and vulnerabilities. When some input is not well-defined accoridng to the spec, then your interpretation is just a random guess, and the next developer will make a different random guess as to what that input means, and so an interoperability problem and potential vulnerability is born. If you reject the invalid input, you will notice the error and thus fix the source of the invalid input to produce input for which the semantics are actually well-defined.
Requirement: Name input box.
Implementation: We'll sanitize the input by rejecting any characters likely to be dangerous if mishandled, like single quotes, or anything else we don't immediately imagine to be useful. If a character turns out to be needed later, that's no problem. We'll just change the list.
Security audit: Passes
Later customer complaint: I can't sign up! — J. O'Brien
Dev team: Sorry, too bad. We'd have to re-audit everything and possibly modify code to allow your last name, because there might be code somewhere that relies on the original sanitization for security. That was the point of sanitizing on input, after all. If you want to sign up, it would be easiest for us if you would just change your name.
So:
Name input field, value "J. O'Brien": accept
JSON parameter, value "{foo:bar}": reject
The context was the idea that you should gracefully accept bad input. If your code considers "J. O'Brien" bad input for a name, then that's the problem, not that it doesn't accept bad input.
The issue is that when developers hear they should "reject bad input" in order to avoid vulnerabilities, they often interpret it as a call to reject any user input that isn't already known to be good. Since user inputs are often free text, like the name field, they wind up forbidding any input they hadn't specifically imagined, which doesn't align with any particular recipient's actual data requirement. It creates false-negative edge cases while only providing illusory help against vulnerabilities.
Examples. PHP: Using mysql_escape_string is a no-no - you will forget to add it one day. Using parametrized queries you won't write unsafe SQL.
.NET Core - Outputting to HTML by default only outputs those chars to HTML which are in predefined UTF range. All other chars will be converted to HTML entities. If you want to output raw, you must explicitly use @Html.Raw https://docs.microsoft.com/en-us/aspnet/core/mvc/views/razor...
I worked at a social media company with one of the largest text-based user-content-stores in the world at the time. Some of the features had input-side encoding and some had output-side encoding. I was there ~10 years after the bad practice of input-side encoding started and it very quickly became too cumbersome to know exactly which fields were encoded with what encoding (and I mean both character encoding and htmlentities / specialchars / specific character stripping / etc). We started getting ridiculous bugs like passwords could not contain '&' characters or logins would fail matching what we had in the DB.
It's not about being perfect. That will never happen. It's about storing exactly what the user submitted (if it is accepted by the POST submission logic) and to correctly encode the output for the correct security context (HTML, XML, JSON, html entities, html attribute, script tag, styles/stylesheet, urls, uploaded filename / file contents, filesystem injection, command injection, etc). These all have different rules. You can unintentionally open yourself to a vulnerability in one if you only expect the output to be displayed in HTML.
What I always do is to exactly specify what is allowed in any input by parsing, schema validation. If it is HTML I run a HTML parser to validate accepted tags & attributes. If it is plain text i validate that there is no HTML in it, etc.
If the input fails the filter then you deny the request.
This has the advantage that you always know what data structures you are storing in the database and that will make future data migrations much easier.
Drawback is that if your filter is too strict then you deny a valid request, however it is easier too loosen a filter later than migrate unwanted/unknown data that you accidentally accepted.
Stored input is also part of your database schema.
And of course, always escape output even if you know the data is "safe".
> Apparently I shouldn't have simplified "paramaterazation of SQL" as "sanitize your input"
It's not a simplification, it's incorrect. An SQL query is output. You are sending data out of your application code, via the SQL library driver.
It may seem like I'm splitting hairs here, but I've seen this distinction misunderstood in this way often enough in situations that severely compromise security.
Some people think of "output" as purely the end product of application flow, and all I/O that happens in between is somehow lumped together as indistinct "input". There's a reason I/O has two separate letters, and the distinction is crucial for securing your application.
TL;DR:
values passed to an I/O function (like stdout, file.write(), response.write(), db.query(), etc.) are all output, those returned from an I/O function (like stdin, file read, db query results or requestObject.getQueryParam()) are input. Sanitize the former, NOT the latter.
Validate the latter (ideally. Though I'd say this is more about stability than security).
What kind of data store can't hold all possible strings of characters? Except perhaps '\0'. I'll make an exception for '\0'.
(EDIT: There is of course the question of what you mean by a character. Sometimes it's an octet. Sometimes it's a Unicode code point.)
Escaping the output however as a term implies you are doing it right, while sanitizing the input could also mean you just replace("DROP", "") etc. (My last name is Dropmann, I know what I am talking about)
This sounds like my spouse saying "you always...". It's obviously not "always"; I think you'll get a better reply when you quantify it. For example, in the last month, how many times did you get a search problem (which was it), a site unavailable or something else (what was it).
You should really do all three:
1) Sanitize input on the server.
2) Use a CSP to prevent your browser from rendering any asset unless specifically whitelisted.
3) Use your front end framework to sanitize text being displayed.
But in any case, this still suggests a complete misunderstanding of the point of that blog post. As far as that blog post's point is concerned, the SQL database is an output of your program. And the whole point is that you need to escape/encode all outputs correctly, but you should not ever sanitize anything.
I think it is totally fair to interpret the article as saying that the database is a program output. And if you interpret it that way, what I said doesn't make sense.
That's just not how I interpreted it.
What I don't understand is the lack of using proper escaping functions when generating SQL. Templating SQL without escaping is the surest way to a SQL injection.
--- Bad
"SELECT * FROM USERS WHERE name = '%'" % (name)
"SELECT * FROM USERS WHERE name = '%'" % sq_esc(name)
# where sq_esc() doesn't add outer ', but escapes anything that needs it
0. Sanitize input. Always, always.
1. Assert pre-condition invariants.*
2. Process.
3. Assert post-condition invariants.*
4. Generate correct output by understanding the output domain.
* Unit tests, smoke tests, integration tests and code-coverage alone are insufficient to cover complex code paths. Fuzzing with asserted invariants is a good way to shake the dust out of hairy code.
What is sql templating used for in the face of parameterized queries?
We did this at reddit. We had basic SQL sanitization on the way in, and then a full pass on the way back to the user. The advantage this gave us is that when someone discovered a new way to hack our sanitization, all we had to do was update the output filter and everything was magically safe.
We didn't have to do full database scans to find all the bad data and change it.
Edit: Apparently I shouldn't have simplified "paramaterazation of SQL" as "sanitize your input". I used the more generic term since I was talking about any kind of data store. But yes, it was of course parameterized.