I was assigned a task to fix the SQL injection flaw reported by a code analysis tool. I am not the original author of the code. I am fairly knowledgeable with SQL.
public int ExecuteNonQuery(string query, SqlParameter[] parameters)
{
using (SqlCommand command = CreateCommand(query, parameters))
{
int rowsAffected = command.ExecuteNonQuery();
log.Debug("rows affected: {0}", rowsAffected);
return rowsAffected;
}
}
and the tool has reported the injection flaw at the line number 3. The ExecuteNonQuery
is being called from so many places.
Here is one such sample:
public void AddDataFrom(IWorkflowStepExecutionStorage storage, bool inverseSourceAndTargetSide)
{
WorkflowStepExecutionStorage storageImpl = storage as WorkflowStepExecutionStorage;
if (storageImpl == null)
{
throw new InvalidOperationException();
}
const string queryTemplate =
@"insert into [$mappingDictionaryTable](sourceObjectId, sourceObject, targetObjectId, targetObject, linkedObjectTypesPairId)
select sourceTable.objectId, sourceTable.systemEntry, targetTable.objectId, targetTable.systemEntry, @linkedObjectTypesPairId from
[$mappingTable] mt
inner join [$sourceImportStorageTable] sourceTable on mt.$sourceIdColumn = sourceTable.id
inner join [$targetImportStorageTable] targetTable on mt.$targetIdColumn = targetTable.id
";
QueryBuilder queryBuilder = new QueryBuilder(queryTemplate);
queryBuilder.AddParameter("$mappingDictionaryTable", this.mappingDictionaryTable.TableName);
queryBuilder.AddParameter("$mappingTable", storageImpl.MappedTable.TableName);
queryBuilder.AddParameter("$sourceImportStorageTable", inverseSourceAndTargetSide ?
(storageImpl.TargetImportStorage as ImportExecutionStorage).ImportTable.TableName :
(storageImpl.SourceImportStorage as ImportExecutionStorage).ImportTable.TableName);
queryBuilder.AddParameter("$targetImportStorageTable", inverseSourceAndTargetSide ?
(storageImpl.SourceImportStorage as ImportExecutionStorage).ImportTable.TableName :
(storageImpl.TargetImportStorage as ImportExecutionStorage).ImportTable.TableName);
queryBuilder.AddParameter("$sourceIdColumn", inverseSourceAndTargetSide ? MappedTable.TargetIdColumn.Name : MappedTable.SourceIdColumn.Name);
queryBuilder.AddParameter("$targetIdColumn", inverseSourceAndTargetSide ? MappedTable.SourceIdColumn.Name : MappedTable.TargetIdColumn.Name);
string query = queryBuilder.GetParametrizedQuery();
SqlParameter[] sqlParams = new[] { new SqlParameter("@linkedObjectTypesPairId", storageImpl.RunConfiguration.LinkedTypesPairConfiguration.Id) };
using (IQcDbConnection dbConnection = this.connectionProvider.CreateDbConnection())
{
dbConnection.ExecuteNonQuery(query, sqlParams);
}
}
Here is the CreateCommand
code:
private SqlCommand CreateCommand(string commandText, SqlParameter[] parameters)
{
SqlCommand retVal = this.connection.CreateCommand();
retVal.CommandText = commandText;
retVal.CommandTimeout = this.commandsTimeout;
retVal.Parameters.AddRange(parameters);
return retVal;
}
Is there any injection risk in this code? If yes, how do I recognize it?