SQL Parametrized query for database backup is reported as sql injection by sonarqube

Question

I have several SQL injection security hotspots reported by SonarQube. I've changed my implementation to use parametrized queries but the issue hasn't been solved. SonarQube is reporting SQL Injection at The following line:

SqlCommand cmd = new SqlCommand(cmdTxt, con);

How may i solve it?

String cmdTxt = String.Format(@"BAKUP DATABASE {0} TO DISK N'{1}'WITH FORMAT,COPY_ONLY,INIT, NAME ='{0} -Full Database Backup', SKIP", databasename, outputfile);

SqlCommand cmd = new SqlCommand(cmdTxt, con);

cmd.Parameters.Add("@database", SqlDbType.VarChar);

cmd.Parameters.Add("outputfile", SqlDbType.VarChar);

cmd.Parameters.["@database"].Value= database;

cmd.Parameters.["outputfile"] .Value= outfile;

Cmd.ExecuteNonQuery();

Problem solved as :

 string sqlQuery = "DECLARE @sql nvarchar (max) = (" +
                                     "SELECT " +
                                     "'BACKUP DATABASE '" +
                                     "+ QUOTENAME(name) " +
                                     "+ ' TO DISK=@outputfile WITH FORMAT,COPY_ONLY,INIT, NAME = @backup_set_name_var, SKIP' " +
                                     "FROM sys.databases " +
                                     "WHERE name = @database); "+
                                     "EXEC sp_executesql " +
                                     "@sql, " +
                                     "N'@backup_set_name_var nvarchar(128), @outputfile nvarchar(128)', "+
                                     "@backup_set_name_var = @backup_set_name_var, "+
                                     "@outputfile = @outputfile; ";
                                     

            using (SqlCommand sqlCmd = new SqlCommand(sqlQuery , _sqlConnection))
            {
                sqlCmd.Parameters.Add("@database", SqlDbType.NVarChar, 128).Value = databaseName;
                sqlCmd.Parameters.Add("@backup_set_name_var", SqlDbType.NVarChar, 128).Value = databaseName + " -Full Database Backup";
                sqlCmd.Parameters.Add("@outputfile", SqlDbType.NVarChar, 128).Value = outputFile;

                sqlCmd.ExecuteNonQuery();
            }

Answer 1

It's hard to say whether allowing a database name as a parameter is in itself dangerous. Arguably, it is, but it is not necessarily SQL injection.

So if you wanted to do this, you could pass this through and create a verified dynamic SQL statement (note the using block, and correct parameter types and sizes):

using(SqlCommand cmd = new SqlCommand(thebelowtext, con))
{
    cmd.Parameters.Add("@database", SqlDbType.NVarChar, 128).Value= database;
    cmd.Parameters.Add("@backup_set_name", SqlDbType.NVarChar, 128).Value= database + " -Full Database Backup";
    cmd.Parameters.Add("@outputfile", SqlDbType.NVarChar, 128).Value= outfile;

    cmd.ExecuteNonQuery();
}

DECLARE @sql = (
SELECT
    'BACKUP DATABASE '
    + QUOTENAME(name)
    + ' TO DISK=@outputfile WITH FORMAT,COPY_ONLY,INIT, NAME = @backup_set_name, SKIP'
FROM sys.databases
WHERE name = @database);

IF (@sql IS NULL)
    THROW 50000, N'Database does not exist', 0;

PRINT @sql;  --For testing in SSMS
EXEC sp_executesql
    @sql,
    N'@backup_set_name_var nvarchar(128), @outputfile nvarchar(128)',
    @backup_set_name = @backup_set_name,
    @outputfile = @outputfile;

---

EDIT It so happens that BACKUP DATABASE allows database names to be parameters anyway. So you don't actually need dynamic SQL at all.

IF NOT EXISTS (SELECT 1
  FROM sys.databases
  WHERE name = @database);
    THROW 50000, N'Database does not exist', 0;

BACKUP DATABASE @database
TO DISK = @outputfile
WITH FORMAT, COPY_ONLY, INIT,
NAME = @backup_set_name, SKIP;