Is this SQL Procedure "injection proof"?

Question

Most all answers and examples of SQL injection are showing some form of dynamic SQL or interpreting parameters as SQL.

I haven't been able to find an example of the "correct" way. Microsoft and Oracle's documentation just shows examples of what not to do.

So, I figured I should ask if this example of a stored procedure was protected against SQL injection attacks.

CREATE PROCEDURE test
    @username = varchar(30)
    @password = varchar(30)
AS
BEGIN
    SELECT *
    FROM credentials
    WHERE username = @username
    AND password = @password;
END
GO

Would this particular procedure be susceptible to SQL injections? I created the procedure and executed it with various attempts to inject SQL, such as EXEC test @password = '0; drop table credentials;', but was unable to do so. I figure I might not be doing the attack correctly.

Answer 1

SQL injection requires that the malicious content be added to (injected into) an SQL query string, which is then parsed by the database engine and executed. As a result, anything that looks like a valid SQL statement will be executed.

The query inside a stored procedure is parsed at the procedure creation time, long before it can receive any content, malicious or not. After that any input will be treated simply as a value for a variable of a certain type, not as an executable instruction. The worst that can happen is a wrong value stored in a column, or the statement failing to execute entirely because of a value overflow or a similar error. No unexpected action is possible.

Similarly, a prepared statement is parsed and compiled before any parameter values can be accepted. When you invoke the prepared statement later, it will only accept the parameters compatible with the statement semantics and treat them as values, not part of the statement.

Answer 2

It's not enough that just the stored procedure is injection-proof. The code that calls the sproc must be secure too.

As a trivial counterexample, consider the following C# code that calls your sproc:

string commandText = string.Format(
  "EXEC test @username = '{0}', @password = '{1}';", 
  username, password
);

SqlCommand command = new SqlCommand(commandText, connection);
command.Connection.Open();
command.ExecuteNonQuery();

Username and password are prompted from an user. Let's say username is bob but password is blaah'; drop database northwind;--. In such a case, the commandText that's sent to SQL Server would look like so:

EXEC test @username = 'bob', @password = 'blaah'; drop database northwind;--'

Although the stored procedure would process bob and blaah parameters just fine without injection, the rest of the code does allow for injection and thus your day will be ruined.

The secure way would be to use parametrized queries. In those, the .Net client library makes sure the values passed to sproc parameters are properly escaped.

SqlCommand cmd = new SqlCommand("sp_Add_contact", con)
cmd.CommandType = CommandType.StoredProcedure;

cmd.Parameters.Add("@username", SqlDbType.VarChar).Value = username;
cmd.Parameters.Add("@password", SqlDbType.VarChar).Value = password;

Answer 3

With dynamic SQL, you first build the SQL statements and then execute them in a later step. But in this case, you are passing parameters to your query. When you execute this statement "EXEC test @password = '0; drop table credentials;'", the where clause for password looks for the value '0; drop table credentials;' in the table. Therefore this procedure is not susceptible to SQL injection.

Answer 4

The procedure itself is fine, it would only be vulnerable if you were doing some kind of sql generation and execution inside it. You are however correct that you are doing the attack incorrectly, but it's not a problem with your procedure.

SQL Injection attacks happen when you execute sql you generate using string manipulation instead of parameters. You don't mention what language you are executing the stored procedure from, but most have libraries that will allow you to specify that you want to execute a stored procedure and pass parameters as values instead of passing a string that does exec myproc 'value1', 'value2'.

If you are doing the latter and generating the entire sql query yourself such as string input1 = "value1", input22 = "value2"; string sql = "exec myproc '" + input1 + "', '" + input2 + "'", then that is where injection occurs. This example would have sql of exec myproc 'value1', 'value2'. This code is vulnerable to sql injection if input1 or input2 is provided by a user or another system. Someone could enter this for input2:

'; drop table mytable; --

The sql string you inject then becomes:

exec myproc 'value1', ''; drop table mytable --'

The apostrophe at the start of the input string closes the apostrophe added by you, the semicolon indicates a new statement, you inject the drop table sql, and the hyphens at the end tell it the rest of the line is a comment so the closing apostrophe added by the code does not cause a syntax error

Your test simply passes a string as the @password parameter to your stored procedure, the fact that the contents look like sql to a human doesn't matter.

EXEC test @password = '0; drop table credentials;'

Answer 5

No. Injection can only happen with dynamic sql. Dynamic SQL uses one of those 2 statements:

exec @querystring

sp_executesql ....

Your proc has none of these, it is thus not dynamic, it is thus not injectable.

Answer 6

Further to the answers above indicating that how one calls the procedure is what is most important, you can make this procedure more secure against SQL injection by:

• make all tables and views no access by the account which runs it;
• ensuring that the account that runs it can run this stored procedure only;
• disabling all other access by the account which runs it.

This works by ensuring that if a SQL injection were to occur, the resulting arbitrary SQL code execution is harmless due to the account not having permissions to do anything other than run that one stored procedure.

Note that this approach does not work for Postgres due to its non-standard way of handling stored procedures in this regard.