18

I made the following stored procedure:

ALTER PROCEDURE usp_actorBirthdays (@nameString nvarchar(100), @actorgender nvarchar(100))
AS
SELECT ActorDOB, ActorName FROM tblActor
WHERE ActorName LIKE '%' + @nameString + '%'
AND ActorGender = @actorgender

Now, I tried doing something like this. Maybe I am doing this wrong, but I want to be sure that such a procedure can prevent any SQL Injection:

EXEC usp_actorBirthdays 'Tom', 'Male; DROP TABLE tblActor'

The image below shows the SQL above being executed in SSMS and results being displayed correctly instead of an error:

enter image description here

Btw, I added that part following the semicolon after the query was done executing. Then I executed it again, but when I checked to see if the table tblActor exists or not, it was still there. Am I doing something wrong? Or is this really injection-proof? I guess what I am trying to ask here also is that is a stored procedure like this safe? Thank you.

  • Have you tried this EXEC usp_actorBirthdays 'Tom', 'Male''; DROP TABLE tblActor' – MarmiK Apr 26 '17 at 7:09
38

This code works properly because it is:

  1. Parameterized, and
  2. Not doing any Dynamic SQL

In order for SQL Injection to work, you have to build a query string (which you are not doing) and not translate single apostrophes (') into escaped-apostrophes ('') (those are escaped via the input parameters).

In your attempt to pass in a "compromised" value, the 'Male; DROP TABLE tblActor' string is just that, a plain-ol' string.

Now, if you were doing something along the lines of:

DECLARE @SQL NVARCHAR(MAX);

SET @SQL = N'SELECT fields FROM table WHERE field23 = '
          + @InputParam;

EXEC(@SQL);

then that would be susceptible to SQL Injection because that query is not in the current, pre-parsed context; that query is just another string at the moment. So the value of @InputParam could be '2015-10-31'; SELECT * FROM PlainTextCreditCardInfo; and that might present a problem because that query would be rendered, and executed, as:

SELECT fields FROM table WHERE field23 = '2015-10-31'; SELECT * FROM PlainTextCreditCardInfo;

This is one (of several) major reason to use Stored Procedures: inherently more secure (well, as long as you don't circumvent that security by building queries like I showed above without validating the values of any parameters used). Though if you need to build Dynamic SQL, the preferred way is to parameterize that as well using sp_executesql:

DECLARE @SQL NVARCHAR(MAX);

SET @SQL = N'SELECT fields FROM table WHERE field23 = @SomeDate_tmp';

EXEC sp_executesql
  @SQL,
  N'SomeDate_tmp DATETIME',
  @SomeDate_tmp = @InputParam;

Using this approach, someone attempting to pass in '2015-10-31'; SELECT * FROM PlainTextCreditCardInfo; for a DATETIME input parameter would get an error when executing the Stored Procedure. Or even if the Stored Procedure accepted @InputParameter as NVARCHAR(100), it would have to convert to a DATETIME in order to pass into that sp_executesql call. And even if the parameter in the Dynamic SQL is a string type, coming into the Stored Procedure in the first place any single apostrophe would automatically get escaped to a double apostrophe.

There is a lesser known type of attack in which the attacker tries to fill up the input field with apostrophes such that a string inside of the Stored Procedure that would be used to construct the Dynamic SQL but which is declared too small can't fit everything and pushes out the ending apostrophe and somehow ends up with the correct number of apostrophes so as to no longer be "escaped" within the string. This is called SQL Truncation and was talked about in an MSDN magazine article titled "New SQL Truncation Attacks And How To Avoid Them", by Bala Neerumalla, but the article is no longer online. The issue containing this article — the November, 2006 edition of MSDN Magazine — is only available as a Windows Help file (in .chm format). If you download it, it might not open due to default security settings. If this happens, then right-click on the MSDNMagazineNovember2006en-us.chm file and select "Properties". In one of those tabs there will be an option for "Trust this type of file" (or something like that) which needs to be checked / enabled. Click the "OK" button and then try opening the .chm file again.

Another variation of the Truncation attack is, assuming a local variable is used to store the "safe" user-supplied value as it had any single-quotes doubled so as to be escaped, to fill up that local variable and place the single-quote at the end. The idea here is that if the local variable is not properly sized, there won't be enough room at the end for the second single-quote, leave the variable ending with a single single-quote that then combines with the single-quote that ends the literal value in the Dynamic SQL, turning that ending single-quote into an embedded escaped single-quote, and the string literal in the Dynamic SQL then ends with the next single-quote that was intended to begin the next string literal. For example:

-- Parameters:
DECLARE @UserID      INT = 37,
        @NewPassword NVARCHAR(15) = N'Any Value ....''',
        @OldPassword NVARCHAR(15) = N';Injected SQL--';

-- Stored Proc:
DECLARE @SQL NVARCHAR(MAX),
        @NewPassword_fixed NVARCHAR(15) = REPLACE(@NewPassword, N'''', N''''''),
        @OldPassword_fixed NVARCHAR(15) = REPLACE(@OldPassword, N'''', N'''''');

SELECT @NewPassword AS [@NewPassword],
       REPLACE(@NewPassword, N'''', N'''''') AS [REPLACE output],
       @NewPassword_fixed AS [@NewPassword_fixed];
/*
@NewPassword          REPLACE output          @NewPassword_fixed
Any Value ....'       Any Value ....''        Any Value ....'
*/

SELECT @OldPassword AS [@OldPassword],
       REPLACE(@OldPassword, N'''', N'''''') AS [REPLACE output],
       @OldPassword_fixed AS [@OldPassword_fixed];
/*
@OldPassword          REPLACE output          @OldPassword_fixed
;Injected SQL--       ;Injected SQL--         ;Injected SQL--
*/

SET @SQL = N'UPDATE dbo.TableName SET [Password] = N'''
           + @NewPassword_fixed + N''' WHERE [TableNameID] = '
           + CONVERT(NVARCHAR(10), @UserID) + N' AND [Password] = N'''
           + @OldPassword_fixed + N''';';

SELECT @SQL AS [Injected];

Here, the Dynamic SQL to be executed is now:

UPDATE dbo.TableName SET [Password] = N'Any Value ....'' WHERE [TableNameID] = 37 AND [Password] = N';Injected SQL--';

That same Dynamic SQL, in a more readable format, is:

UPDATE dbo.TableName SET [Password] = N'Any Value ....'' WHERE [TableNameID] = 37 AND [Password] = N';

Injected SQL--';

Fixing this is easy. Just do one of the following:

  1. DON'T USE DYNAMIC SQL UNLESS ABSOLUTELY NECESSARY! (I'm listing this first because it really should be the first thing to consider).
  2. Properly size the local variable (i.e. should be twice the size as the input parameter, just in case all characters passed-in are single-quotes.
  3. Don't use a local variable to store the "fixed" value; just put the REPLACE() directly into the creation of the Dynamic SQL:

    SET @SQL = N'UPDATE dbo.TableName SET [Password] = N'''
               + REPLACE(@NewPassword, N'''', N'''''') + N''' WHERE [TableNameID] = '
               + CONVERT(NVARCHAR(10), @UserID) + N' AND [Password] = N'''
               + REPLACE(@OldPassword, N'''', N'''''') + N''';';
    
    SELECT @SQL AS [No SQL Injection here];
    

    The Dynamic SQL is no longer compromised:

    UPDATE dbo.TableName SET [Password] = N'Any Value ....''' WHERE [TableNameID] = 37 AND [Password] = N';Injected SQL--';
    

Notes about the Trunction example above:

  1. Yes, this is a very contrived example. There is not much one can do in only 15 characters to inject. Sure, maybe a DELETE tableName to be destructive, but less likely to add a back-door user, or change an admin password.
  2. This type of attack probably requires knowledge of the code, table names, etc. Less likely to be done by random stranger / script-kiddie, but I did work at a place that was attacked by a rather upset former employee who knew of a vulnerability in one particular web page that nobody else was aware of. Meaning, sometimes attackers do have intimate knowledge of the system.
  3. Sure, resetting everyone's password is likely to be investigated, which might tip the company off that there is an attack happening, but it might still provide enough time to inject a back-door user or maybe get some secondary info to use/exploit later.
  4. Even if this scenario is mostly academic (i.e. not likely to happen in the real world), it's still not impossible.

For more detailed information related to SQL Injection (covering various RDBMS's and scenarios), please see the following from the Open Web Application Security Project (OWASP):
Testing for SQL Injection

Related Stack Overflow answer on SQL Injection and SQL Truncation:
How safe is T-SQL after you replace the ' escape character?

  • 2
    Oh, thank you very much, this is a great answer. I understand now. I would really like to see the technique you mentioned at the end too where the attacker tries to fill up the input field with apostrophes, if you can find it. Thanks in advance. I am going to keep this open, in case you do not find it, I will choose this as the answer. – Ravi Nov 17 '15 at 3:29
  • 1
    @Ravi I found the link but it no longer gets to the article as they are all now archived. But I added some info and useful links and I am still trying to find the article within those archives. – Solomon Rutzky Nov 17 '15 at 4:03
  • 1
    Thank you srutzsky, I will read the OWASP article and the tests for injection. If I remember correctly, 'mutillidae', the vulnerable web app for security testing, has SQL injection that I had performed at college with the 'OR 1 = 1' string which in mutillidae resulted in me logging into the web app as the admin I think. That's when I was first introduced to SQL injection. – Ravi Nov 17 '15 at 14:51
  • 1
    I am not able to view the .chm file too, but thank you for this perfect answer and for all the helpful links including the one from stackoverflow and the one from OWASP. I read it today and learned a lot from this. – Ravi Nov 18 '15 at 5:16
2

The simple matter is that you're not confusing data with command at all. The values for the parameters are never treated as part of the command, and therefore are never executed.

I blogged about this at: http://blogs.lobsterpot.com.au/2015/02/10/sql-injection-the-golden-rule/

  • Thank you Rob, I have this one bookmarked for reading later in the day. – Ravi Nov 17 '15 at 14:53

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.