Secure way of accessing SQL Stored Procedures from Entity Framework Core

Question

I am mid 'Pen Test' and trying to find the "most secure" way to use Entity Framework Core to Stored Procedures. You would think it would be obvious, but please read on.

Background

Using this document on using Raw SQL and having read Whats new in .NET Core 2.0 my initial code used string interpolation (showed as safe in the 'what's new' document).

However, the Pen Test successfully managed a sql injection attack against it.

Offending code example:

dbResults = _datacontext.Blogs.FromSql($"myStoredProcedure @parmeter1 = {parmeter1String}");

So, I changed the code to use params, but they also broke through this:

dbResults = _datacontext.Blogs.FromSql("myStoredProcedure @parmeter1 = {0}", parmeter1String);

and they broke this (though maybe I wasn't cleaning enough - at least I stopped exec):

dbResults = _datacontext.Blogs.FromSql("myStoredProcedure @parmeter1 = {0}", parmeter1String.ToCleanedSqlString());

So is the answer to use SqlParameter (not shown in any of the above docs/examples)?

dbResults = _datacontext.Blogs.FromSql("myStoredProcedure @parmeter1 = {0}", new SqlParameter("@parmeter1", parmeter1String));

or is there a better way?

Some definitive guidance would be appreciated please.

EDIT Following comment:

Important addition: The stored procedure does execute dynamic sql, but that procedure is not in my application and I don't have control over it. I have to call it.

Presumably the first example is missing a leading $" ? Both examples should most certainly not be vulnerable to injection, do you have an example input string that breaks the parameterization? Does the underlying SP use the given param in dynamic SQL in any way? — Alex K.
– Alex K., Commented Feb 18, 2019 at 15:27
Any of these will bind command parameter, so I don't see how the sql injection attack against it succeeds, except if your stored procedure is using the @parmeter1 to execute dynamic SQL. Can we see the SP in question? — Ivan Stoev
– Ivan Stoev, Commented Feb 18, 2019 at 15:28
That's the real issue then, a nicely parameterized value can still cause injection if its (mis)used in dynamic SQL. The way to mitigate this depends on what validation logic you can apply to the input before sending it to the procedure & what the SP does with the value it receives. The responsibility for fixing this lies with the DBA/developer of the procedure. — Alex K.
– Alex K., Commented Feb 18, 2019 at 15:38
Following @AlexK. 's comment, I have provided the information to the DBA, who has (following this proof) agreed to redo the stored procedure to use parameters in the dynamic sql rather than strings sommarskog.se/dynamic_sql.html#good_practices. Really appreciate the input. Thank you. — JJJCoder
– JJJCoder, Commented Feb 18, 2019 at 17:08

JJJCoder · Accepted Answer · 2019-03-25 09:31:12Z

Following comments, I thought I should add a conclusion (Thank you @AlexK via comment).

If a stored procedure is written using non-parameterized dynamic sql, it is not possible for Entity Framework to protect it. This is not a fault of Entity Framework (which is, in general, very good at sql protection).

Should you find yourself in a similar situation, the stored procedure should be re-written using parameterised dynamic sql. A very good explanation can be found here.

To Elaborate.

There is no way for Entity Framework to protect sql like this:

CREATE PROCEDURE search_orders @custid   nchar(5)     = NULL,
                               @shipname nvarchar(40) = NULL AS
DECLARE @sql nvarchar(4000)
SELECT @sql = ' SELECT OrderID, OrderDate, CustomerID, ShipName ' +
              ' FROM dbo.Orders WHERE 1 = 1 '
IF @custid IS NOT NULL
   SELECT @sql = @sql + ' AND CustomerID LIKE ''' + @custid + ''''
IF @shipname IS NOT NULL
   SELECT @sql = @sql + ' AND ShipName LIKE ''' + @shipname + ''''
EXEC(@sql)

But this will be safe:

CREATE PROCEDURE search_orders @custid   nchar(5) = NULL,
                               @shipname nvarchar(40) = NULL AS
DECLARE @sql nvarchar(4000)
SELECT @sql = ' SELECT OrderID, OrderDate, CustomerID, ShipName ' +
              ' FROM dbo.Orders WHERE 1 = 1 '
IF @custid IS NOT NULL
   SELECT @sql = @sql + ' AND CustomerID LIKE @custid '
IF @shipname IS NOT NULL
   SELECT @sql = @sql + ' AND ShipName LIKE @shipname '
EXEC sp_executesql @sql, N'@custid nchar(5), @shipname nvarchar(40)',
                   @custid, @shipname

As far as I can tell, any of the code samples from the question would be valid 'safe' EF Code (though the extra work on the extension method would have been surplus to requirements).

Update

Following this, I contacted Microsoft and created a PR, the basis of which has been accepted. The warning message on the official documentation is now more detailed, hopefully reducing the chance of someone having the same problems.

Collectives™ on Stack Overflow

Secure way of accessing SQL Stored Procedures from Entity Framework Core

1 Answer 1

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

Comments

Your Answer

Sign up or log in

Post as a guest

Related