C#•8mo ago

Preventing SQL Injections with string based queries

I'm working with an SDK to execute queries in a mssql database. This SDK offers a method that is required to execute those queries, but this method only takes a string. And string based queries are damn prone to sql injection attacks. So I want to write a middle piece method that takes in a SqlCommand object in which I can set parameters and returns the assembled query as string. Please give me feedback on it:

public static string GetParsedSqlCommand(SqlCommand cmd)
    {
        string sql = cmd.CommandText;
        foreach (SqlParameter param in cmd.Parameters)
        {
            string placeholder = param.ParameterName;
            string value = FormatSqlValue(param);
            sql = sql.Replace("@" + placeholder, value);
        }
        return sql;
    }

    public static string FormatSqlValue(SqlParameter param)
    {
        if (param.Value == DBNull.Value || param.Value == null)
        {
            return "NULL";
        }

        switch (param.SqlDbType)
        {
            case System.Data.SqlDbType.NVarChar:
            case System.Data.SqlDbType.VarChar:
            case System.Data.SqlDbType.Char:
            case System.Data.SqlDbType.NChar:
            case System.Data.SqlDbType.Text:
            case System.Data.SqlDbType.NText:
            case System.Data.SqlDbType.UniqueIdentifier:
            case System.Data.SqlDbType.Xml:
                return $"'{param.Value.ToString().Replace("'", "''")}'";
            case System.Data.SqlDbType.Date:
            case System.Data.SqlDbType.DateTime:
            case System.Data.SqlDbType.DateTime2:
            case System.Data.SqlDbType.SmallDateTime:
            case System.Data.SqlDbType.Time:
                return $"'{((DateTime)param.Value).ToString("yyyy-MM-dd HH:mm:ss", CultureInfo.InvariantCulture)}'";
            case System.Data.SqlDbType.Bit:
                return ((bool)param.Value) ? "1" : "0";
            default:
                return param.Value.ToString();
        }
    }

public static string GetParsedSqlCommand(SqlCommand cmd)
    {
        string sql = cmd.CommandText;
        foreach (SqlParameter param in cmd.Parameters)
        {
            string placeholder = param.ParameterName;
            string value = FormatSqlValue(param);
            sql = sql.Replace("@" + placeholder, value);
        }
        return sql;
    }

    public static string FormatSqlValue(SqlParameter param)
    {
        if (param.Value == DBNull.Value || param.Value == null)
        {
            return "NULL";
        }

        switch (param.SqlDbType)
        {
            case System.Data.SqlDbType.NVarChar:
            case System.Data.SqlDbType.VarChar:
            case System.Data.SqlDbType.Char:
            case System.Data.SqlDbType.NChar:
            case System.Data.SqlDbType.Text:
            case System.Data.SqlDbType.NText:
            case System.Data.SqlDbType.UniqueIdentifier:
            case System.Data.SqlDbType.Xml:
                return $"'{param.Value.ToString().Replace("'", "''")}'";
            case System.Data.SqlDbType.Date:
            case System.Data.SqlDbType.DateTime:
            case System.Data.SqlDbType.DateTime2:
            case System.Data.SqlDbType.SmallDateTime:
            case System.Data.SqlDbType.Time:
                return $"'{((DateTime)param.Value).ToString("yyyy-MM-dd HH:mm:ss", CultureInfo.InvariantCulture)}'";
            case System.Data.SqlDbType.Bit:
                return ((bool)param.Value) ? "1" : "0";
            default:
                return param.Value.ToString();
        }
    }

19 Replies

Buddy•8mo ago

$sqlinjection

MODiX•8mo ago

Always parametrize queries!⠀ Do not concatenate the query, like in this example:

// Do NOT do this
string query = "SELECT * FROM users WHERE username='" + UserName + "';";
...

// Do NOT do this
string query = "SELECT * FROM users WHERE username='" + UserName + "';";
...

Instead, always parameterize your queries. Look up the documentation for your database library. If you are using System.Data.SqlClient, refer to this example.

Imgur

TurwaithOP•8mo ago

have you read my question?

Buddy•8mo ago

Oh, I see what you mean What SDK is it? I'd just find an alternative if one exists

TurwaithOP•8mo ago

There is no alternative. I'm writing addons for an existing software that provides this specific SDK

Buddy•8mo ago

Yikes

TurwaithOP•8mo ago

yep

Buddy•8mo ago

First option could be to contact the author of said SDK and bring forward the issues.

TurwaithOP•8mo ago

The author is a multi billion dollar company, so that's probably not gonna happen. Which makes the issue existing in their SDK even worse

Buddy•8mo ago

I'd at least try, but yeah. It's bad!

TurwaithOP•8mo ago

The SDK is a mess in many other places as well Do you have some feedback on my code though? I'd at least want to make queries secure for my customers

David_F•8mo ago

@Turwaith are you the one constructing the queries that you pass to the SDK? I mean why did you choose SqlCommand as input? Did you consider EF Core for constructing queries and then using .ToQueryString() method or something similar?

TurwaithOP•8mo ago

Yes I am constructing the queries, but the parameters often come in via an API endpoint I provide. I chose SqlCommand since this is the method I usually use when executing queries on sql databases I control when using C#. It has the option to add parameters and it's easy to use I did not consider that (yet) no

Buddy•8mo ago

If you do decide not to use it, sanitizing queries isn't super easy. https://www.researchgate.net/publication/367069019_Prevention_of_SQL_Injection_Using_a_Comprehensive_Input_Sanitization_Methodology According to the paper some databases (like MySQL) can also decode hexadecimal and interpret it as a string

TurwaithOP•8mo ago

Hmm I guess it would be the best option to go for efcore then, even though it seems kinda overkill to add an entire new dependency for just one single method

David_F•8mo ago

@Turwaith also I am not sure you prevent SQL injection in your code when case System.Data.SqlDbType.Text: and you invoke param.Value.ToString().Replace("'", "''") what happens when I inject malicious text in param? I mean, you need to check whether SqlParameter type prevents such abuse

TurwaithOP•8mo ago

From what I've read, SqlCommand and SqlParameter are a solid way to prevent injection in .net. I'm always open to be taught otherwise though

Pobiega•8mo ago

They are when executed as an SqlCommand with parameters, but I dont think that safety extends to when you construct the full SQL query yourself based on its components.

David_F•8mo ago

I've checked with Bing Copilot it says that your string formatting does not prevent SQL injection. Try this

SqlCommand cmd = new SqlCommand("SELECT * FROM Customers WHERE City = @City");
cmd.Parameters.Add(new SqlParameter("@City", "London'; DROP TABLE Customers; --"));
string sql = GetParsedSqlCommand(cmd);

SqlCommand cmd = new SqlCommand("SELECT * FROM Customers WHERE City = @City");
cmd.Parameters.Add(new SqlParameter("@City", "London'; DROP TABLE Customers; --"));
string sql = GetParsedSqlCommand(cmd);

also in your code at line sql = sql.Replace("@" + placeholder, value); there is a redundant "@" + placeholder already includes @ prefix

Gaming

Programming

Preventing SQL Injections with string based queries