Can I protect against SQL injection by escaping single-quote and surrounding user input with single-quotes?

84,667

Solution 1

First of all, it's just bad practice. Input validation is always necessary, but it's also always iffy.
Worse yet, blacklist validation is always problematic, it's much better to explicitly and strictly define what values/formats you accept. Admittedly, this is not always possible - but to some extent it must always be done.
Some research papers on the subject:

Point is, any blacklist you do (and too-permissive whitelists) can be bypassed. The last link to my paper shows situations where even quote escaping can be bypassed.

Even if these situations do not apply to you, it's still a bad idea. Moreover, unless your app is trivially small, you're going to have to deal with maintenance, and maybe a certain amount of governance: how do you ensure that its done right, everywhere all the time?

The proper way to do it:

  • Whitelist validation: type, length, format or accepted values
  • If you want to blacklist, go right ahead. Quote escaping is good, but within context of the other mitigations.
  • Use Command and Parameter objects, to preparse and validate
  • Call parameterized queries only.
  • Better yet, use Stored Procedures exclusively.
  • Avoid using dynamic SQL, and dont use string concatenation to build queries.
  • If using SPs, you can also limit permissions in the database to executing the needed SPs only, and not access tables directly.
  • you can also easily verify that the entire codebase only accesses the DB through SPs...

Solution 2

Okay, this response will relate to the update of the question:

"If anyone knows of any specific way to mount a SQL injection attack against this sanitization method I would love to see it."

Now, besides the MySQL backslash escaping - and taking into account that we're actually talking about MSSQL, there are actually 3 possible ways of still SQL injecting your code

sSanitizedInput = "'" & Replace(sInput, "'", "''") & "'"

Take into account that these will not all be valid at all times, and are very dependant on your actual code around it:

  1. Second-order SQL Injection - if an SQL query is rebuilt based upon data retrieved from the database after escaping, the data is concatenated unescaped and may be indirectly SQL-injected. See
  2. String truncation - (a bit more complicated) - Scenario is you have two fields, say a username and password, and the SQL concatenates both of them. And both fields (or just the first) has a hard limit on length. For instance, the username is limited to 20 characters. Say you have this code:
username = left(Replace(sInput, "'", "''"), 20)

Then what you get - is the username, escaped, and then trimmed to 20 characters. The problem here - I'll stick my quote in the 20th character (e.g. after 19 a's), and your escaping quote will be trimmed (in the 21st character). Then the SQL

sSQL = "select * from USERS where username = '" + username + "'  and password = '" + password + "'"

combined with the aforementioned malformed username will result in the password already being outside the quotes, and will just contain the payload directly.
3. Unicode Smuggling - In certain situations, it is possible to pass a high-level unicode character that looks like a quote, but isn't - until it gets to the database, where suddenly it is. Since it isn't a quote when you validate it, it will go through easy... See my previous response for more details, and link to original research.

Solution 3

In a nutshell: Never do query escaping yourself. You're bound to get something wrong. Instead, use parameterized queries, or if you can't do that for some reason, use an existing library that does this for you. There's no reason to be doing it yourself.

Solution 4

I realize this is a long time after the question was asked, but ..

One way to launch an attack on the 'quote the argument' procedure is with string truncation. According to MSDN, in SQL Server 2000 SP4 (and SQL Server 2005 SP1), a too long string will be quietly truncated.

When you quote a string, the string increases in size. Every apostrophe is repeated. This can then be used to push parts of the SQL outside the buffer. So you could effectively trim away parts of a where clause.

This would probably be mostly useful in a 'user admin' page scenario where you could abuse the 'update' statement to not do all the checks it was supposed to do.

So if you decide to quote all the arguments, make sure you know what goes on with the string sizes and see to it that you don't run into truncation.

I would recommend going with parameters. Always. Just wish I could enforce that in the database. And as a side effect, you are more likely to get better cache hits because more of the statements look the same. (This was certainly true on Oracle 8)

Solution 5

I've used this technique when dealing with 'advanced search' functionality, where building a query from scratch was the only viable answer. (Example: allow the user to search for products based on an unlimited set of constraints on product attributes, displaying columns and their permitted values as GUI controls to reduce the learning threshold for users.)

In itself it is safe AFAIK. As another answerer pointed out, however, you may also need to deal with backspace escaping (albeit not when passing the query to SQL Server using ADO or ADO.NET, at least -- can't vouch for all databases or technologies).

The snag is that you really have to be certain which strings contain user input (always potentially malicious), and which strings are valid SQL queries. One of the traps is if you use values from the database -- were those values originally user-supplied? If so, they must also be escaped. My answer is to try to sanitize as late as possible (but no later!), when constructing the SQL query.

However, in most cases, parameter binding is the way to go -- it's just simpler.

Share:
84,667

Related videos on Youtube

Patrick
Author by

Patrick

Updated on July 17, 2020

Comments

  • Patrick
    Patrick almost 4 years

    I realize that parameterized SQL queries is the optimal way to sanitize user input when building queries that contain user input, but I'm wondering what is wrong with taking user input and escaping any single quotes and surrounding the whole string with single quotes. Here's the code:

    sSanitizedInput = "'" & Replace(sInput, "'", "''") & "'"
    

    Any single-quote the user enters is replaced with double single-quotes, which eliminates the users ability to end the string, so anything else they may type, such as semicolons, percent signs, etc., will all be part of the string and not actually executed as part of the command.

    We are using Microsoft SQL Server 2000, for which I believe the single-quote is the only string delimiter and the only way to escape the string delimiter, so there is no way to execute anything the user types in.

    I don't see any way to launch an SQL injection attack against this, but I realize that if this were as bulletproof as it seems to me someone else would have thought of it already and it would be common practice.

    What's wrong with this code? Is there a way to get an SQL injection attack past this sanitization technique? Sample user input that exploits this technique would be very helpful.


    UPDATE:

    I still don't know of any way to effectively launch a SQL injection attack against this code. A few people suggested that a backslash would escape one single-quote and leave the other to end the string so that the rest of the string would be executed as part of the SQL command, and I realize that this method would work to inject SQL into a MySQL database, but in SQL Server 2000 the only way (that I've been able to find) to escape a single-quote is with another single-quote; backslashes won't do it.

    And unless there is a way to stop the escaping of the single-quote, none of the rest of the user input will be executed because it will all be taken as one contiguous string.

    I understand that there are better ways to sanitize input, but I'm really more interested in learning why the method I provided above won't work. If anyone knows of any specific way to mount a SQL injection attack against this sanitization method I would love to see it.

    • SantiBailors
      SantiBailors almost 9 years
      @BryanH Admitting not to understand how the commonly accepted wisdom applies to a specific case and asking for an example about such specific case is not hubris, it's humbleness. Getting annoyed when someone asks for an example of why the commonly accepted wisdom is right on the other hand might come across as arrogant. Reasoning by specific examples is often a great way to investigate and learn. The way the OP went about this doubt was very useful for my understanding of the subject, especially when he explained the answer he found.
    • 3therk1ll
      3therk1ll over 4 years
      @patrik Just came across this as I'm working on the same piece of code but trying to escape the string and nest a query. Did you ever figure it out?
    • Patrick
      Patrick over 4 years
      @3therk1ll it's best not to try, you're better off using parameterized SQL: blog.codinghorror.com/…
    • 3therk1ll
      3therk1ll over 4 years
      @Patrick, I'm approaching it from the attackers perspective!
  • Patrick
    Patrick over 15 years
    Thanks for the response! I know that attack would work for a mySQL database but I'm pretty sure that MS SQL Server won't accept a backslash as an escape character (I tried it). Several google searches did not reveal any other escape characters, which really made me wonder why this wouldn't work.
  • Nick Johnson
    Nick Johnson over 15 years
    You can still use parameter substitution even if you're building your own queries.
  • JeeBee
    JeeBee over 15 years
    You should build the SQL statement string from scratch, but still use parameter substitution.
  • AviD
    AviD over 15 years
    No, NEVER build your SQL statements from scratch.
  • Patrick
    Patrick over 15 years
    That could work, but again, how could they get that code to execute when all user input is surrounded by single quotes? A specific line(s) of code that would be able to inject SQL into the above code would be very helpful. Thanks!
  • Jørn Jensen
    Jørn Jensen about 15 years
    After posting, I decided that AviD's post covers this, and in more detail. Hopefully my post will still be of help to someone.
  • Brian
    Brian almost 14 years
    When used correctly, dynamic SQL and string concatenation can be used safely with parameterized queries (i.e. with sp_executesql instead of EXEC). That is, you can dynamically generate your SQL statement so long as none of the concatenated text comes from the user. This also has performance benefits; sp_executesql supports caching.
  • AviD
    AviD almost 14 years
    @Brian, well duh :). But in reality, how often do you see programmers do that? Moreover, the typical scenario where dynamic SQL is "needed", requires the user input as part of the query (supposedly). If you could do sp_executesql, you wouldn't (usually) need the dynamic sql in the first place.
  • systempuntoout
    systempuntoout almost 12 years
    What if you have to deal with something like "Google Fusion tables" where, afaik, there's no any abstraction library available that supports its dialect? What would you suggest?
  • Patrick
    Patrick over 11 years
    I finally ran into a situation that made me realize that it is possible to use unicode to sneak past the string replacement. The input text was typed into Word, which changed the apostrophe from the straight-down version to a "curly" apostrophe (which looks more like a comma), which was not affected by the string replacement but was treated as a string delimiter by SQL Server. Thanks for the answer AviD (and everyone else)!
  • BryanH
    BryanH over 11 years
    And when you miss that ONE case on ONE input, you're pwnd.
  • El Ronnoco
    El Ronnoco over 11 years
    @AviD I think what Brian is saying is where the SqlCommand Text doesn't contain input directly from the user. It will instead contain parameter placeholders such as @name which can then be set to anything you like without the risk of injection. I am discounting someone building something dumb like SqlCommand.Text="EXEC(@userinput)" which would obviously deserve to be destroyed.
  • AviD
    AviD over 11 years
    @ElRonnoco sure, but I don't discount that, since I've seen it in the wild more times than you'd think...
  • MickeyfAgain_BeforeExitOfSO
    MickeyfAgain_BeforeExitOfSO over 10 years
    "Some people, when confronted with a problem, think "I know, I'll use regular expressions." Now they have two problems."
  • tom.dietrich
    tom.dietrich over 10 years
    @mickeyf I know this is a common sentiment but honestly regular expressions are pretty awesome once you grep them.
  • Eric J.
    Eric J. about 10 years
    The link to the SQL Smuggling PDF is now broken. Can you update the link?
  • Michael Fredrickson
    Michael Fredrickson over 9 years
    @AviD I updated the link to the SQL Smuggling PDF that you wrote to the only version I could find online... please let us know if there's another location for your paper.
  • AviD
    AviD over 9 years
    Thanks @MichaelFredrickson!
  • SantiBailors
    SantiBailors almost 9 years
    @tom.dietrich It always depends on the real life situation. F.ex. regexpr syntax is not standard so in general I would advise against using regexpr in contexts where different systems are integrated to work together. This is because different regexpr engines evaluate regexprs differently, and more importantly this hard fact gets usually downplayed or ignored which can lead developers to not care about these incompatibilities until they get bitten. There are plenty of such incompatibilities; see f.ex. regular-expressions.info/shorthand.html (search for flavors in that page).
  • Hogan
    Hogan about 8 years
    It is like you didn't read all the other answers to this 8 year old question, since any number of these answers point out his method fails to stop injection if the attacker simply uses unicode characters.
  • miroxlav
    miroxlav about 8 years
    @Hogan – I did, but I think there is extra value in my question. I have much experience and testing behind what I wrote. I know using query parameters is better, but I also fully understand the situation when someone must avoid it due to various reasons (e.g. employer demands to keep the old way). In this case, I think my answer is very comprehensive and has higher value than answers saying "just don't do that", because it shows the way through. Show me other answers here which show the same way and I'll consider deleting mine.
  • Hogan
    Hogan about 8 years
    Ok, when (not if) your system gets compromised please come back and delete this answer.... or you could use a parametrized query.
  • miroxlav
    miroxlav about 8 years
    @Hogan – I have no problem to do it :) But currently I claim there is no known way to get around this if you keep the 4 rules I posted. If you really think there is a way around it, then just point out where.
  • andrewf
    andrewf almost 8 years
    Down-voted because answer does not address question. Question is about escaping strings in SQL. When you escape an arbitrary string (as the questioner is trying to do, in order to deal with unsanitised data), you can’t just replace problematic characters with arbitrary other ones; that corrupts data. (Also, a single quote IS an apostrophe (at least in ASCII).)
  • Roman Starkov
    Roman Starkov almost 7 years
    "Call parameterised queries only": yeah... until you run into JSON_MODIFY which ONLY accepts string literals for path. Facedesk...
  • Shayne
    Shayne about 5 years
    Its probably worth noting that generally its smart to use ORMs if performance isn't a primary concern, just make sure its a good one (Ie don't use Active Record pattern ones, use Data mappers, and for god sake learn and use database side constraints). The reason for this is generally the people who write the ORMs are much more experienced with SQL security and are likely to be recieving vunerability notices and acting on them. Many of these have decent validation frameworks built in, as well as the eternally useful migration systems.
  • Shayne
    Shayne about 5 years
    Bad advice hombre. any interpolation can be defeated.
  • miroxlav
    miroxlav about 5 years
    @Shayne - any source to back this claim? So I and other readers can learn from it. You comment is not much different from the above ones, which claim that without any source or pointer.
  • Scott Smith
    Scott Smith over 3 years
    The question is not which is wiser, it is about exactly how a particular solution actually fails. If you don't know, then you don't have an answer to this quetion.
  • Jake
    Jake over 2 years
    I am using SQL Server. I know about parametrised sql etc but I have edge situation that may need dynamic sql. Access to the input field is highly restricted. If the input is stripped to just A-Z characters (no spaces) is SQL injection possible. Does anyone have examples?
  • vpalmu
    vpalmu over 2 years
    I'm sorry, we have just reached a case where all of these are infeasible. It's escape the literal and embed or replace SQL server with a SQL dialect that allows it.
  • vpalmu
    vpalmu over 2 years
    Can't reproduce.
  • Andrew P.
    Andrew P. about 2 years
    Good information, but how does this address the question?!