Is it safe method for C# against sql injection?

string sqlDeclare = $"DECLARE  @number nvarchar(MAX)  SET @number = '%{sNumber}%' ";


string sqlFilter = "";

if (!string.IsNullOrEmpty(Number))
{
   sqlFilter  = $" and [table].[number] like @number ";
}


 string sql = sqlDeclare   " SELECT [table].[*] ";
 sql  = " WHERE [table].[state] = 0 and [table].[column1] <> 3 AND [table].[id] > 0  ";

 if (!string.IsNullOrEmpty(sqlFilter))
 {
     sql  = sqlFilter;
 }
 sql  = " order by datein";

P.S. I can't use Parametr.Add()

CodePudding user response：

Is it safe method for C# against sql injection?

if sNumber actually was a numeric type, like int or decimal then I'd say "maybe" because it's pretty hard to inject SQL into a number, but at the end of the day, all you've done is change from concatenating a string value directly into an sql query, which is the classic fail:

"SELECT * FROM t WHERE x = "   str

into putting a string value into a db variable that you then use in an sql. It means the select query won't be the place where the injecting is done, but it just moves the injection point earlier:

Think what happens when sNumber is a value like '; DROP TABLE students;--

DECLARE  @number nvarchar(MAX)  SET @number = '%';
DROP TABLE Students;--%' 
SELECT ...

If you truly are restricted to using this function (it needs throwing away, tbh) then you'll have to do your best at preventing injection by sanitizing the input. In this case you say you're expecting a document number like 2-12 so your C# should look like

if(!Regex.IsMatch(userInput, @"^\d -\d $"))
  throw new ArgumentException("Bad input value, expecting document number like 2-12");

var dt = Execute4Table($"SELECT * FROM t WHERE x = '{input}');

In a general sense, it would be clunky, but you could perhaps do some wrapper function that takes N parameters and serializes them to JSON, then you pass them to sqlserver as a json string and get SQLS to deser them into N parameters that you use in the query. The two serializers should encode any user provided values in such a way that they are interpreted as data rather than code. You could also do a similar thing with base64 if your sqlserver is too old for json. To some extents this isn't much different to replacing all ' with '' but with a set/deser approach you're handing off responsibility for that encoding to a well tested library, which is probably the only thing you can do if you are rather hamstrung by this method you're forced to use

CodePudding user response：

It would be safe if you type check the sNumber variable and you made sure it has no string dangerous data (your sNumber could have 0';TRUNCATE TABLE [table];-- and that would be SQL Injection). If you check if your snumber is integer, you would be safe on your string interpolation.

Otherwise it would not be safe. It is best to avoid string interpolation/string concatenation.

int checkedNumber;
if (Int32.TryParse(out checkedNumber, sNumber))
{
    string sqlDeclare = $"DECLARE  @number nvarchar(MAX)  SET @number = '%{sNumber}%' ";
    string sqlFilter = "";
    if (!string.IsNullOrEmpty(Number))
    {
       sqlFilter  = $" and [table].[number] like @number ";
    }
     string sql = sqlDeclare   " SELECT [table].[*] ";
     sql  = " WHERE [table].[state] = 0 and [table].[column1] <> 3 AND [table].[id] > 0  ";

    if (!string.IsNullOrEmpty(sqlFilter))
    {
        sql  = sqlFilter;
    }
    sql  = " order by datein";
}