Do Stored Procedures Protect Against SQL Injection?


When I’ve asked people about their strategies for preventing SQL injection, one response is sometimes “I use stored procedures.” But, stored procedures do not, by themselves, necessarily protect against SQL injection. The usefulness of a stored procedure as a protective measure has everything to do with how the stored procedure is written. Write a stored procedure one way, and you can prevent SQL Injection. Write it another way, and you are still vulnerable. This post will look at one common pitfall that can leave stored procedures vulnerable to SQL injection.

For starters, let’s suppose we have the following (admittedly contrived) login script:

<form method="post" action="injection.php" enctype="multipart/form-data" >
    Username:<input type="text" name="Username" id="Username"/></br>
    Password:<input type="text" name="Password" id="Password"/></br>
    <input type="submit" name="submit" value="Submit" />
</form>

<?php
if(isset($_POST['Username']))
{
    $username = $_POST['Username'];
    $password = $_POST['Password'];

    // Connect to the server.
    $server = "MyServer\sqlexpress";
    $uid = "userName";
    $pwd = "password";
    $database = "ExampleDB";
    $connectionoptions = array("Database" => $database
                               , "UID" => $uid
                               , "PWD" => $pwd);
    $conn = sqlsrv_connect($server, $connectionoptions);

    // Execute a parameterized query.
    $params = array($username, $password);
    $stmt = sqlsrv_query($conn, "{call VerifyUser( ?, ? )}", $params);

    // If a row is returned, we have a username-password match.
    if(sqlsrv_has_rows($stmt))
    {
        echo "Welcome.";
    }
    else
    {
        echo "Invalid password.";
    }
}
?>

First, note that this example is very similar to the example I showed in this post: What’s the right way to prevent SQL Injection in PHP Scripts? Also note that it uses a parameterized query, which is a very important in preventing SQL injection (see the linked-to post). The difference here is that I’m using a stored procedure, VerifyUser, to determine if a user is valid.

Note: Calling stored procedures using canonical syntax (as shown in the example above) is the recommended practice when using the SQL Server Driver for PHP.. For more information about canonical syntax, see Calling a Stored Procedure.

As I said in the introduction, how that stored procedure is written can determine whether or not my script is vulnerable to SQL injection. Let’s take a look at two ways to write that stored procedure…

The wrong way

Suppose the VerifyUser stored procedure was created by dynamically building a SQL string within the stored procedure, like this:

CREATE PROCEDURE VerifyUser
    @username varchar(50),
    @password varchar(50)
AS
BEGIN
    DECLARE @sql nvarchar(500);
    SET @sql = 'SELECT * FROM UserTable
                WHERE UserName = ''' + @username + '''
                AND Password = ''' + @password + ''' ';
    EXEC(@sql);
END
GO

Now, when I execute my PHP script with this input…

image

…the SQL that the stored procedure executes is this:

SELECT * FROM UserTable WHERE UserName = 'Brian' --' AND Password = 'any password'

 

The last half of the query is commented out! As long as my user name matches some user name in the database, I’m in.

By building the SQL query as a string in the stored procedure and concatenating parameter values in that string, I run the same risks that are inherent in concatenating parameter values in application code – I’m vulnerable to SQL injection. I admit that building a query dynamically as shown in the stored procedure above is somewhat contrived, but it is meant to show what is possible (and what NOT to do). Fortunately, avoiding the problem above is easy…

The right way

Now suppose the VerifyUser stored procedure was created like this:

CREATE PROCEDURE VerifyUser
    @username varchar(50),
    @password varchar(50)
AS
BEGIN
    SELECT * FROM UserTable 
    WHERE UserName = @username 
    AND Password = @password;
END
GO

Now, an execution plan for the SELECT query  exists on the server before the query is executed. The plan only allows our original query to be executed. Parameter values (even if they are injected SQL) won’t be executed because they are not part of the plan. So, if I submit a username like I did in the example above (Brian’ –), it will be treated as user input, not SQL code. In other words, the query will look for a user with this password instead of executing unexpected SQL code.

The lesson is the same as it was in this post: don’t concatenate user input with SQL. Use prepared statements.

Once again, that’s my 2 cents.

Thanks.

-Brian

Share this on Twitter

Comments (13)

  1. Anonymous says:

    I know it's subtle, but I really like the way you call a stored procedure a 'plan.'  It puts me in the right mindset when creating SPs to think of them as plans and not queries.

    As always, so well worded, Brian!

  2. Anonymous says:

    Using a decent O/RM will prevent the SQL injection attack (by sanitizing input) and remove the burden of writing all those sprocs..

  3. Anonymous says:

    What about input validations and strictly disallowing some injectable characters? Although, a Client side validation will only work for the honest user, A combination of serverside validation will help. Again, Trading off speed or performance for security!

  4. @David – Thanks. I probably should not have been so subtle about the "plan". I was referring to query plans (en.wikipedia.org/…/Query_plan) which can improve performance as well as protect against injection.

    @Owolabi – IMHO, validation should be done on both the client and the server. Obviously, if speed is the most important factor in your application, you favor one over the other, but that's going to depend on the application. Just my 2 cents.

  5. Anonymous says:

    The author did not comment about input validations because it is irrelevant to the specific topic being discussed, i.e. whether stored procedures can protect against SQL injection. Input validation is a necessary thing, but it's beside the point in the context of this article.

  6. Anonymous says:

    Won't passing parameters into dynamic SQL also protect against SQL injection?  I would have expected this to form one of the 'right way' examples

  7. Anonymous says:

    I would suggest that there is a very simple rule that protects agains SQL injection:

     NEVER build dynamic SQL out of EXTERNAL input

    Dynamic SQL is fine, as long as all the building blocks are inside your procedure/function/query. For example, you might have to select from a different combination of tables depending on some user-input parameter. That's perfectly safe. Trouble comes when you allow the user input to be pasted into your query.

    In the example, the SQL injection vulnerability has simply been moved out of the PHP script and into the stored procedure. The stored procedure is guilty of building dynamic SQL out of something (the input parameters) whose value is set OUTSIDE the stored procedure.

    If you're the developer working on that stored procedure, you can't be sure who or what will end up calling your code. For this reason, SQL injection is not just for web developers – it's every developer's responsibility. System variables, input parameters, results from other functions – all these could contain some horrible SQL that you don't want in your query.

    Sure, you can check the external input for suspect characters, try to strip out punctuation and so on. But string parsing is notoriously inefficient, and how can you be sure you've covered every possibility? What happens if the input you've been given – like the password in the example – is allowed to contain strings like ' or — ?

    Follow the simple rule and you should be completely safe.

  8. Thanks for the questions and comments. I think Andy P summed things up nicely. I tried to point out what he is saying in the post, but I didn't quite say it as eloquently. 🙂 As Andy P points out, in my example the SQL injection vulnerability is just moved from the PHP script to the stored procedure. His NEVER build dynamic SQL out of EXTERNAL input is a nice, simple summary.

  9. Anonymous says:

    Never building sql by concatenating EXTERNAL input is fine advice … as far as it goes. The problem is: sometimes developers forget the source of their inputs. Secondary sql injection is a technique whereby malicious code is inserted into a database table where it lies in wait for a naive developer to forget that the source of the data in that table was EXTERNAL.

    The proper answer to the question of how to prevent sql injection is: always use parameters instead of concatenation.

    Attempting to sanitize/cleanse input is not a technique for preventing sql injection: it should be more properly thought of as a technique for attempting to detect injection attempts, and should never be the only technique used to secure your application. It is  the first layer in a multi-layer defense.

  10. Anonymous says:

    Bob, I was reluctant to make my (rather long) comment even longer by discussing secondary SQL injection. My view is that the same rule still applies, because anything you're getting from a table is EXTERNAL to your stored procedure (or function, or query, or whatever). Just to clarify, when I talk about EXTERNAL input, I don't just mean somthing that comes in from the internet, or that comes from the user: I mean anything that comes from outside the scope of the query, function or procedure you're writing.

    Perhaps we can refine the rule to:

    —   NEVER concatenate dynamic SQL out of EXTERNAL input.

    —   EXTERNAL input can ONLY be used as a PARAMETER.

  11. Anonymous says:

    Nice

  12. Anonymous says:

    Nice

  13. Anonymous says:

    Great article, I especially like the near-even mixture of commentary and code. Quick, to-the-point and illustrative.

    @Andy P, I think as a general rule that's a great guideline. I would make this one case for an exception to the second point:

    EXTERNAL input can ONLY be used as a PARAMETER…

    …unless the source has been verified to come FULLY from trusted sources.

    Case in point, you can't dynamically select a table using a parameter. I use a type system wherein components can aggregate another set of fields by providing type id and record id. Because the type table is 100% under control of the system and authorized developers, there wont be SQL injection coming from there.

    On the contrary, I've wanted a solution that doesn't rely on query building. While I think trusted sources are valid to concatenate in this case, I'd be all ears for a better way.