Give me parameterized SQL, or give me death

Useing parametised queries is wisdom from God :slight_smile:
Relying on concatenated SQL is plain stupid period. Would you leave the keys in your front door and go on an extended holiday? A lot of good folk here are wondering if there are ways to make it safe …There are none.
Most times the parameter comes from a query string or http post, these are easily hijacked (especially query string). You can remove “dangerous items”, escape returned values all you like; what about encoding? Most database engines will encode SQL to string regardless of the encoding the app employs. Canny hackers employ this and can send in innocous strings that your code will surely miss …and the honey pot is open - your code doesn’t know about it and neither do yo until it is too late.
If you doubt it, lets experiment; show me your app and I will shock you with its weaknesses.
There are too many uneducated Developers out there making life difficult for others.

I will like to add to my last post. You might decide to ascii encode all db entries to help with your check, but what about if your db is backing a multi-lingual app or you need to globalise? That way you can hardly avoid unicode.
In any event hackers can encode the dangerous entry several times …Believe me, your db is clever enough to decode back to the dangerous entry and your’e zonked all over!
Anyways encoding is just one of many means of executing both SQL Injection and XSS.
Educate yourself on these things.

Those of you against parameterized sql are missing the point.

Yes, you can write a perfect input sanitisation routine. It is possible, people have managed it in the past and will in the future. However, developers get it wrong time and time again. Features are added and tweaked, people change code they don’t have the time to understand sufficiently, possible attack techniques are forgotten about and new ones are created.

As a security professional, I come across this time and time again. So when some inexperienced junior comes along and introduces a vulnerability and I discover it, I’m not going to be looking at them as the problem, I’m going to be pointing my finger at you.

Hi all,

first of all I like parameterized queries a lot. It greatly reduce possible runtime errors and - once one got used to it - it is much more comfortable to develop because all this concatinations and type case and localization issues are widely gone…

It’s definitly the case that two different execution plans are built:

Think about an NC Index for C2. Selectivity C2: 43.600 distinct c2, 1-131 rows out of 950000 per C2, what is great selektivity, also for C2 value with 131 rows

select C1 from T1 where C2 = @myParam
select C1 from T1 where C2 = ‘myString’

Regardless if you put the statement in a stored procedure or execute it via ADO.NET SqlCommand object via sp_executeSql the query plan results in a index SCAN. It’s NOT ONLY a parameter sniffing issue and also statistics are updated. The plan is always wrong!?! This is crap for every possible value for C2.

When I execute the SQL text directly (without params) the index seek is done as expected! Always as expected…

When executed via stored procedure it helps to use a local variable:

create procedure p1
(
@myParam varchar(5)
)
as
declare @myParam1 varchar(5) – helper var
set @myParam1 = @myParam
select C1 from T1 where C2 = @myParam – wrong, index SCAN
select C1 from T1 where C2 = @myParam1 – correct, index SEEK

So, unfortunatly i must say that the optimizers behavior is often more understandable when using direct unparameterized SQL.

If anybody has an idea about this issue, please let me know!

Regards
Mario

After years of using string concatenation it is a blessing to have Parameterized SQL. I routinely serialize objects to SQL and dealing with string, integer, and binary types is a breeze now.

I’m just redoing a lot of code for parametised SQL instead of SPs, so far quite simple and easy to change, also easily customisable for other database types including some w/o stored procs.

As for the O’Brien problem, we regularly replace the ’ character with ` - the one next to the number 1 on your (well my UK) keyboard. Unlike ’ its inert in SQL, looks the same on paper or the screen, and is easy to catch.

Hi,

Fell across this site while having to convert reams of dynamic sql to parametised. The solution to the in clause issue is to write a function that splits the argument string returns them as a temp table.

Not convinced on performance yet but it works.

This allows you to write something like

“SELECT val FROM table WHERE id In (SELECT * FROM ufn_ArgSplitter(@strIDs))”

the function looks like

CREATE FUNCTION dbo.ufn_ArgSplitter
(
@String VARCHAR(1024)
)
RETURNS
@ParsedList TABLE
(
arg VARCHAR(50) COLLATE database_default
)
AS

BEGIN
DECLARE @ITEM VARCHAR(50), @Pos INT

SET @String = LTRIM(RTRIM(@String))+ ','
SET @Pos = CHARINDEX(',', @String, 1)

IF REPLACE(@String, ',', '')  ''
BEGIN
WHILE @Pos  0
BEGIN
    SET @ITEM = LTRIM(RTRIM(LEFT(@String, @Pos - 1)))
    IF @ITEM  ''
	BEGIN
	    INSERT INTO @ParsedList (arg) 
		VALUES (@ITEM)
	END
	SET @String = RIGHT(@String, LEN(@String) - @Pos)
	SET @Pos = CHARINDEX(',', @String, 1)

END
END	
RETURN

END

For completeness sake here’s an example where removing quotes does not help:

SELECT email, passwd, login_id, full_name
FROM members
WHERE login_id = + userinput + ;

userinput: 1 or 1 = 1

SELECT email, passwd, login_id, full_name
FROM members
WHERE login_id = 1 or 1 = 1;

Since parameterized queries seem to be handled by an object in all the languages/frameworks I’ve seen them in, I’m kind of confused - how exactly are they performed as textual SQL?

From what I can gather, I imagine the actual SQL query would resemble something like this:

SET @var1 = ‘foo’;
SET @var2 = ‘bar’;
SELECT col FROM table WHERE var1 = @var1 AND var2 = @var2;

Is this correct as a very basic example, or is the structure different?

Yes, Jung, you seem to have the basic idea.

The only time I ever dynamically concatenate a sql string is when I’m build an in list of ids. I always use a list of integers for this. Tell me how that could get hacked. phhhhh

Joe Beam:
It’s arrogance like yours, tunnel-vision, and failure to use proper standards that creates a world of problems in the first place. You think you’re smarter than any hacker out there, so you assume what you write is unbreakable. Then you quit, move on, whatever, and the next guy comes in and does something -you- would never have done, and WHAM! Someone hacks in. Is it your replacement’s fault? Or your own for not doing it properly in the first place?

This page still comes up high in some searches, despite having been written in 2005!

In the meantime, cmd.Parameters.Add() has been deprecated; use cmd.Parameters.AddWithValue() instead with the same parameters.

In my experience… ADO 2.6:
strSQL = " SELECT * From Users Where UserName = ? "

Set cmd = New ADODB.Command
cmd.ActiveConnection = "_connectionString"
cmd.CommandText = strSQL
cmd.CommandType = adCmdText

cmd.Parameters.Append cmd.CreateParameter("@UserName", adVarChar, adParamInput, 20, UserName)

Set rsLocal = cmd.Execute

Any other examples?

God this is old, but like many have mentioned all over the net, old habits die hard. I plan to answer the questions and issues still left open on this discussion for those who are still searching about this, since even still in late 2014 my college campuses (yes, I attended 2 colleges thus far) teach SQL, ASP, and web developement (using anything from HTML, to ASPNET, to PHP), and yet There has never been anything mentioned about parameterized/prepared SQL statements. Okay, now to the huge analytic breakdown you have been waiting for:

Jill asks:

I need to get a string from the user… but
it’s never going to need to contain a single-quote character.

Here’s the deal Jill: much like my current project that will NEVER accept any special characters in the user input fields, and the data is very well restricted, making your application layer of your code refuse to accept ANY harmful character is a viable alternative in terms of sql-injection prevention as an end-result. Let me clarify with my specific example:

your HTML page creates a form, of which a text field exists that posts its contents to your next php in the superglobal ‘host’, on your next php page. Because it is a textbox, the field data will be posted as a string, meaning PHP will specifically treat the entire contents as the variable content, and NOTHING will attempt to interpret the contents at this point:

<?php     $host = $_POST['host'];     if (!preg_match("#^[a-z0-9.-]+$#i", $host) || preg_match("#^[.-]#", $host) || preg_match("#[.-]$#", $host) || preg_match("#--#")) {                 $host = 'NULL';     }     ?>

What this has just done is check through the variable $host, from its beginning to its end, and determined the following:

1). Does the string contain anything other than a-z, 0-9, period (“.”), or a dash (“-”)? OR
2). Is the first character of the string a dash or period? OR
3). Is the last character of the string a dash or a period? OR
4). Is there any occurrence of “–” in the string?

if any of these conditions is true, the IF evaluates to true, and the code rewrites the $host variable to ‘NULL’, which can be used to check for errors (or just simply use a $host_entry_error variable assigned as true for later checking).

On your error checking area down the script, you would run ALL of your user inputs this way and determine whether any were invalid entries. If there were, your PHP script should just recreate the form, blank out the invalid entries, and display some bold and red warning about why it failed, and DON’T RUN ANY QUERIES! Even if the username needs to be checked for availability, it first needs to pass script validation as a valid entry, then the select statement can be used. I use multi-point error checking per variable so the user can see everything that was found wrong the first time, and each bad variable is mentioned, not just the first bad one per submission. Can’t tell you how many times the username guidelines weren’t given, and my username was too long/short, contained invalid characters, could not begin with a number, cannot use a dash, but can use a dot, etc. That’s my personal preference on my site, but then again, I manually approve new user accounts, no automation involved, so much more control.

Now, as long as we all know that the characters a-z, 0-9, and the dash and period are harmless when applied in this fashion (meaning no --, no start/end with - or .), we just whitelist these cases and say NOGO to anything else. This is effective sanitation, validation, and security of accepting user input on a limited scope. In short, you will not be able to inject SQL into this field if this level of error checking is performed. Period. For those who want to troll this method, please keep reading.

Kender, and anyone else who tried without quotes to inject SQL:

Yes, if the text field is not validated for bad characters and sequences, and it is just immediately tossed into concatenation  of a SQL query, you can easily inject it. What is often being asked is this:

If my app only takes a-z and 0-9, can you inject SQL with just this?

The answer is NO! Since SQL statements require a specific syntax, you can only inject SQL using 1 of 3 possible methods:

1 - introduce an extra quote in order to break the string encapsulation, adding your own WHERE condition (uses ’ )
2 - use a semicolon to end the first query, and then start your own using the still active connection (uses ; )
3 - use escape characters to generate junk data propogation whenever the DB uses that value (uses \ )

There are many other ways to sneak in these characters past a multipart character encoding, but the most surefire way to prevent even this is to not blacklist characters (you will not get them all, unless you really like including 99.999% of EVERY Character encoding set by hand…for every time you set string content match checking! Just make it simple, use the whitelist if you need to do it this way.

What I am about to say must be read CAREFULLY, as I am not against parameterized SQL since I actually understand it, and I will explain it (no, it wasn’t actually explained here HOW or WHY it works as opposed to other deprecated methods).

My philosophy here is that “He who does not understand how AND why something works should not be in the position to make decisions of whether it is implemented.” … bluntly put, if you can’t explain how exactly something works, then you have no business pushing it onto other people, especially if you want to call it a standard when it is still not even taught in college. Trolls please continue.

Those of you who blindly embrace prepared statements without understanding ALL of their implications will be the bain of Software Engineering existence, because there are times when having something and not needing it is not better than covering a small scope of usage and saving resources and time, especially when legacy support is necessary on a tight budget. As an IT Specialist (and I specialize in a lot of areas), I have experienced the need to push a client to something they should be doing, but in the end, if they don’t want it, they won’t do it. For one such client, I suggested they upgrade their systems from Windows Server 2003 to something newer. Their decade-old hardware and server OS was not going to cut it much longer, so Linux was my suggestion after they replace the hardware to help reduce overall costs to a notable amount. Unfortunately, some work needed to be done to their current Web setup since they were using a VERY insecure ASPNET cookie-cutter web template, which was vulnerable via their very own anonymous-allowing FTP server, and even their ADO connection details were listed in their headers… I’ve heard o bad setups, but this was a true example of coding horror. Long story short, they decided to stick with what they had because it “works well enough to let them work with it or around it”. To this day, they are still extremely vulnerable, and refuse to fix their issues the right way. End of bad business client story.

For M_T:

It’s arrogance like yours, tunnel-vision, and failure to use proper
standards that creates a world of problems in the first place.  You
think you’re smarter than any hacker out there, so you assume what you
write is unbreakable.  Then you quit, move on, whatever, and the next
guy comes in and does something -you- would never have done, and WHAM!  
Someone hacks in.  Is it your replacement’s fault?  Or your own for not
doing it properly in the first place?

First, it’s idiocy like yours that actually thinks this is standardized just because you have accepted it without recompense or fully understanding it, demonstrated by your lack of any technical explanation of how this “standard” actually works.

Second, much of what has been written in order to protect your online purchases was written by great programmers, but even they make mistakes, or other programs cause problems to occur within another. The heartbleed bug went unnoticed for 2 years before it was mentioned AND patched for openssl…well, and here I though college grads in software engineering were infallable. These things happen in big businesses all of the time. Take the current ObamaCare Signup servers, why do they suck so much? They were developed by [arguably] reputable software engineers with all of the necessary knowledge, understanding, and experience, yet they look like a high-school dropout had a whack at it. I’ve seen better working mods for games go better than this. So when this happens, and you walk away from your setup, two things are very likely:

You wrote some application that skated the edge of vulnerability using arbitrary (but still valid) methods of mitigation, and the new guy who replaced you made a small change without compensating for this? Personally, I think the new guy deserves a pink slip because he broke rule number 1 of software maintenance: you better know what your changes will do globally before you implement them! The engineers have it easier, they get to design the code. The maintainers have the hardest job of all, they have to UNDERSTAND EVERYTHING that was being done when it was written. This is why the best thing a programmer can learn to do it DOCUMENT your code.

Regardless of whether your method was barely secure or was security-through-obscurity, as the new guy who takes over the maintenance and future development of an application, YOU are responsible for understanding the implications of what changes YOU make. Even if the technique that was first used is removed and no longer supported, it’s YOUR JOB now to identify this and do something about it.

Just for you trolls,

Is it your replacement’s fault?  Or your own for not doing it properly in the first place?

As clearly stated above, its the replacement’s fault if they make changes to a system and it goes down or gets hacked as a result. That’s how the legal dept and the Uppers will see it, so tough. ALWAYS KNOW WHAT YOU ARE DOING BEFORE IT GOES LIVE! we have sandboxes and cloning capability for systems now, use it! As for doing it properly the first time, I absolutely agree, but we all can’t be Jesus, now can we M_T? Even us who hold Master of Science degrees in CIS are bound to make a decision based on our supers, owners, and budget from time to time. Just remember, [INSERT FECECIOUS WORD HERE] runs downhill, and the guy who left when it was still working will never get blamed by anyone but you.

Chris has the right idea for the older method of building queries with sanitation and validation:

As for the O’Brien problem, we regularly replace the ’ character with ` -
  the one next to the number 1 on your (well my UK) keyboard. Unlike ’
its inert in SQL, looks the same on paper or the screen, and is easy to
catch.

When your database is not complex enough to warrant the need to include the possiblity of EVERYTHING, a simple substitution of grave for apostrophe does well enough, especially since it is only for cosmetic necessity. If Mr. O’Brien doesn’t like this method because it makes his apostrophe look different, well (queue troll massacre here), I have a very much needed suffix in my legal name, and as such I find it a complete data nightmare when my name and my father’s name cannot be distinguished in some business database, especially when said business is “not” sharing my personally identifiable data with someone else’s database. Many databases still don’t have a suffix column for a person’s name. Really? How are you going to know who’s who when all you get is a phone number, a first and last name, and the fact that they are a Military Veteran? NEWSFLASH! Both my Father and I are U.S. Military Disabled Veterans, and we have THE SAME NAME! Hence the Jr.! But no, these “VA” affilliates don’t get “personally identifiable” information, just our first and last and phone, and even if we were interested in what they have to say, we can never tell which one they are requesting to speak with. This is just one example. Can you believe what would happen if our bank called us, but cannot verify date of birth, age, last 4 of account, etc. because they have to have me/him verify it first? Are they calling about the house (Cannot disclose that info without verification of your identity)? Are they calling about a credit card of ours(Cannot disclose that info without verification of your identity)? See how this can be a problem?

So, If Mr. O’Brien can’t handle this, just have him call me. At least my father and I served in different branches and held different ranks…My old SgtMaj has this same problem since his father also retired at the same rank in the Marine Corps. Go figure.

black_adder:

Who the hell does guarantee that “smart” objects can’t contain an exploit?

Now here is a person who is doing the right thing; he won’t trust it unless he can see HOW and WHY it works the way it works. He trusts this PDO method of prepared SQL as much as he trusts his anonymous user input, which is apparently not at all. Bravo for boldly stating this black_adder, because it is incredibly true. Millions of businesses put their faith in openSSL for years, and we now find out it had an undetectable vulnerability that allowed anyone to slowly bleed out memory contents of a server, and over time obtain even the server’s secret private key and gain access to the entire server. Trust what you know, and what you have no choice in doing so otherwise. In the end, Man made computers, so they are only as accurate and secure as we make them. If you are writing programs on faulty API’s, then it’s not your fault, it’s the API’s…but if you knew the API was faulty, then its both your fault.

codinghorror:

This password doesn’t cause any SQL injection problems at the time of insertion; it gets written to the database as-is.

Later, when the application constructs a SQL string using the password
field, IT ASSUMES THE PASSWORD FIELD IS A SAFE STRING, and the injection
  takes effect at that time. Not quite as effective, but still a valid
attack in many cases.

BerenW:

I agree with your example in the case of quote escaping, but fail to see
  how it would cause issues (beyond failing to match the altered
password) in the case of quote stripping.

First off, WTF are you doing not hashing passwords stored on a database? Cleartext passwords in a database is just asking to have your dbserver hijacked and eradicated. I hope you don’t store customer credit card details anywhere without hashing either. Password submissions should NEVER be a problem with SQL injection since you should be using a hashing algorithm of some sort to change the password into a character sequence that is safe for SQL storage. If you aren’t doing this, you are very vulnerable for privilege escalation. All one would need to do is get the ability to just look at your user’s table and see the admin account username and password once, then establish their own connection to your DB, then they can create their own Admin account with GRANT ALL, and they can ruin you and you would never know. They only need to passively copy the DB contents and your customers will be suing you for credit card fraud and identity theft. Case in point, LifeLock (Lifelock Dinged $12 Million for Deceptive Business Practices | WIRED), search and read from “Lifelock also promised customers that sensitive data they provided the company to perform its protection services”.

Well, since I am getting tired of just going on and on (yes, hard to believe after reading all of this, huh?), I will wrap up with the points of conjecture. Here is how parameterized SQL statements work vs. an app-hardened concatenation like I alluded to in the beginning of this post.

First, the app-hardened version. The app goes through steps to ensure that only character sequences that are known to be benign to SQL outside of data entry are allowed to be passed to the SQL construction. This method passes the sanitized and safe contents into the SQL statement as part of a string, like so:

<?php     $email = $_POST['email']     sanitize($email)     if ($email == '!BAD_DATA!') {       // don't do SQL, reload page with error message     } else {       ('SELECT email, username FROM TABLE WHERE email = $email;')     }     ?>

The reason SQL injection is theoretically still possible (although unlikely as long as your app continues to prevent those characters and sequences that can create such an injection) is because you are still passing the field data content as a string into a larger string, which will then be interpreted by the SQL interpreter as a whole. If your field data contains characters that result in something like you should have already seen (if not, google SQL injection examples). If at any time your app fails to prevent these characters, such as other backdoors or an oversight in your allowed characters and sequences, your DB is at risk. This is why some of these guys are still saying you should not continue to take the risk and hope for the best. But they still failed to explain why prepared statements are any better. I will do it for them.

Now for how Parameterized SQL statements work (and to kill off the remaining trolls). When a SQL statement was submitted to the SQL server interpreter, it basically ran the entire thing as it saw it. What prepared SQL statements do is it gives out the SQL statement, but it will be missing certain pieces. The engine that prepares the statement is able to discover what the statement will be doing without the actual data. When it determines, say, a SELECT will be preformed, on specified columns, on a specified table, and even that it will filter with the WHERE keyword, it now knows that the SQL query will look for certain columns where the specified data matches an unspecified-at-this-time value. Much like how HTML Form $_POST data works, no matter what was in the textbox when the form was submitted and posted to the next page, the content of the field was sent strictly as data, and could not be interpreted any further on its way into the superglobal $_POST. This is what the placeholders :label or ? do in the prepared statements; they say to the preparing engine “Here is the SQL query you are going to run, and I will give you the data for the placeholders later. When I give it to you, you use it strictly as data, and don’t try to interpret its contents.” When the query is accepted by the preparer, it bypasses the SQL interpreter by doing the interpretation ITSELF.

function ShowHost($host, $mysqli) {
        $stmt = $mysqli → prepare(“SELECT host, ip_address FROM SERVER_LIST WHERE host = ?”);
        $stmt → bind_param(“s”, $host);
        $stmt → execute();

After we gave the prepare() method the statement without the value we were filtering by, it figured out what we wanted it to do, and when we used bind_param() method to further tell the preparer that the next paramter to our previous statement is going to a string (noted by the argument “s”), and the data it will recieve, but not interpret when executing the SQL query, is found in the second argument (the php variable $host). Yes it really is this simple when you have your packages set up to utilize it, most of which will if they are current and up-to-date. Just be sure to specify the proper settings if you decide to use a PDO in your dbconnect php include so it only has to be done once for your whole site.

If you have been thinking or are now thinking “Okay, that’s clear enough, I can let anything into my database with this and I’m totally secured now!” This is true on 1st order injection, where the data is initially entered, but you are still vulnerable. I’ll summarize:

You can prevent the injection from being done by using the prepare() and bind_param() methods properly, and this will ensure the string contents are not interpreted by the SQL query interpreter. HOWEVER, be careful, and this goes right back to Troll M_T, and be aware that no matter how successful you were in safely storing a string that is otherwise dangerous SQL code elsewhere, you are still at risk for 2nd order injection if you EVER use a SQL query to concatenate or utilize the contents of this record’s data from within a string literal, whether from within your app or strictly from a SQL query console on the server.

(php - Are PDO prepared statements sufficient to prevent SQL injection? - Stack Overflow)

On top of this, SQL injection is just one aspect of your DB security, you still should create users specific for your site connections. I personally use a separate user for each major action for not only DDL, but also DML. The user that has permission to create a table will not actually have the same permission to drop it, alter it, insert into it, nor even select. Every user is totally isolated in its capability. Some DB engineers would look at that and go “Huh?”, but there are some who will see why that is done. This effectively helps further prevent SQL injection in areas where a query is being run by a user who cannot execute the injected content. This really narrows the likelihood that an injectable SQL query will work on a given user, even after 1st and 2nd order preparations take place.

There are times when doing prepared statements is better than what you were already doing, and there are times when it is beyond the scope of the project. If your project is not the latter, it can’t hurt to slowly add these prepared statements, especially if you are using PHP. Including a mass script that can take your data, query actions, and other information as parameters is the easiest way to make it available to your entire site without breaking anything. At first you might have lots of unused sanitizing methods, but you can still pass the output of those methods along to the preparering script as a transitional system.

Just remember, even with prepared statements, sanitized inputs, validation of entries, separation of privileges and duties, server role and environment isolation, and whatever else is done to mitigate or prevent attacks, you will still be vulnerable to 0-day vulnerabilities that are not known by you or your system. So, ALWAYS BACKUP YOUR DATA! Your customers will be glad they only lost minutes to hours of data instead of all of it.

And for you trolls who think there is no such thing as “not within the scope of the project”, take an introduction to OOP using Java at your nearest community college, and ask if you can use advanced error checking or use a switch statement in your first two assignements. Okay trolls, fire away.

Link to “stored procedures versus dynamic SQL argument” was http://www.codinghorror.com/blog/archives/000117.html should now be https://blog.codinghorror.com/stored-procedures-vs-ad-hoc-sql/

Is it possible to update the original blog post?

1 Like

The method

public SqlParameter Add(string parameterName, object value); has been deprecated, I believe it should be AddWithValue nowadays in the example.

public SqlParameter AddWithValue(string parameterName, object value);

Warning CS0618 ‘SqlParameterCollection.Add(string, object)’ is obsolete: 'Add(String parameterName, Object value) has been deprecated. Use AddWithValue(String parameterName, Object value).

Example with AddWithValue for .Net 4.5.1 and above:

        using (var conn = new SqlConnection(connectionString))
        {
            conn.Open();
            const string query = "SELECT email, passwd, login_id, full_name " +
                       "FROM members WHERE email = @email";
            using (var cmd = new SqlCommand(query))
            {
                cmd.Parameters.AddWithValue("@email", email);
                var reader = cmd.ExecuteReader();
                // do something with the reader
            }
        }

Link to the Microsoft documentation with another example:
https://docs.microsoft.com/en-us/dotnet/api/system.data.sqlclient.sqlparametercollection.addwithvalue?view=netframework-4.5.1#System_Data_SqlClient_SqlParameterCollection_AddWithValue_System_String_System_Object_