Skip to main content

Validating user supplied input

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?

NJMR
  • 161
  • 4