SQL injection vulnerability veracode c#

Question

Veracode report is showing a SQL injection flaw for the below query.

private const string DropDatabaseTemplate = @"DROP DATABASE [{0}]";
ExecuteNonQuery(connection, string.Format(DropDatabaseTemplate, databaseName));

private static int ExecuteNonQuery(SqlConnection connection, string commandText)
        {
            using (var command = new SqlCommand(commandText, connection))
            {
                return command.ExecuteNonQuery();
            }
        }

they suggested using parameterized prepared statements. What would be my approach to remove this security vulnerability

Thanks in advance.

Ans : You can simply avoid security vulnerability with this

 private static void ExecuteNonQuery(SqlConnection connection, string commandText)
        {
            using (var command = new SqlCommand("exec sp_executesql @sqlCommandText", connection))
            {
                command.Prepare();
                command.Parameters.Add("@sqlCommandText", SqlDbType.NVarChar);
                command.Parameters["@sqlCommandText"].Value = commandText;
                command.ExecuteNonQuery();
            }
        }

Does this answer your question? How to give ADO.NET Parameters — lordvlad30
– lordvlad30, Commented May 6, 2020 at 6:12
While it's true this code is vulnerable, you can't parameterize it. In SQL, you can only parameterize data, not identifiers. The only thing you can do is make sure the database actually exists before attempting to drop it - and that is to prevent leaking information to users in case of an exception. — Zohar Peled
– Zohar Peled, Commented May 6, 2020 at 6:13
That specific statement cannot be parametrized, so the suggestion you received, despite being generally correct, in this case cannot be applied — Cleptus
– Cleptus, Commented May 6, 2020 at 6:15
Having said that, I would recommend against having this method option in your code base. Dropping a database should be a design time operation, not a run time operation. This is true for all database structure changes, BTW. Of course, there could be situations where this rule doesn't apply, but there are very few such situations. — Zohar Peled
– Zohar Peled, Commented May 6, 2020 at 6:16

Caius Jard · Accepted Answer · 2020-09-05 07:43:23Z

1

I've never tried it, but I suspect it will work:

    private static void DropDbNamed(SqlConnection connection, string name)
    {
        using (var command = new SqlCommand("EXEC @q", connection))
        {
            command.Parameters.AddWithValue("@q", $"DROP DATABASE [{name}]"); 
            var command.ExecuteScalar();
        }
    }

_{Note: Joel's standard "stop using AddWithValue" doesn't apply here}

answered Sep 5, 2020 at 7:43

Caius Jard

75k6 gold badges61 silver badges95 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

Ден Денис · Accepted Answer · 2020-05-06 07:41:16Z

0

How it could look like.

private const string DropDatabaseTemplate = @"DROP DATABASE [{0}]";


private static int ExecuteNonQuery(SqlConnection connection, string commandText)
{
    string dbNamesQuery_ = @"SELECT [name]
                FROM sys.databases d
                WHERE d.database_id > 4";

    DataTable tableNames = new DataTable();
    using (var command = new SqlCommand(dbNamesQuery_, connection))
    {
        SqlDataReader dataReader_ = command.ExecuteReader();
        tableNames.Load(dataReader_);        //allow you dynamically load actual list DB, but you can fill table manually.
        //find exactly same name of DB that user requared.
        var rowsData_ = tableNames.Select(String.Format("name = '{0}'", commandText));        
        if (rowsData_.Length == 1)        //it will be prevent any kind of injection.
        {
            command.CommandText = String.Format(DropDatabaseTemplate, commandText);
            return command.ExecuteNonQuery();
        }
        else
        {
            return -1;
        }
    }
}

answered May 6, 2020 at 7:41

Ден Денис

342 silver badges9 bronze badges

3 Comments

Zohar Peled Over a year ago

Why not simply SELECT [name] FROM sys.databases WHERE [name] = @databaseName?

Ден Денис Over a year ago

@Zohar Peled I believe that "databaseName" could be another place for vulnerability

Zohar Peled Over a year ago

Not if it's a parameter like I wrote it. Parameterized queries protects against SQL injection. That's kinda their main point.

Collectives™ on Stack Overflow

SQL injection vulnerability veracode c#

2 Answers 2

Comments

3 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

3 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related