search

pumasecurity / puma-scan

Puma Scan is a software security Visual Studio extension that provides real time, continuous source code analysis as development teams write code. Vulnerabilities are immediately displayed in the development environment as spell check and compiler warnings, preventing security bugs from entering your applications.
https://www.pumascan.com
Mozilla Public License 2.0
446 stars 80 forks source link

SEC0108 warning with recommended overload #51

Open brettpostin opened 5 years ago

brettpostin commented 5 years ago

The rule documentation for SEC0108 recommends...

To ensure calls to vulnerable EF methods are parameterized, pass parameters into the statement using the method’s second argument: params object[] parameters.

However the following code still causes a warning for usage of ExecuteSqlCommand and SqlQuery...

public void ExecuteProcedure(string procedureName, List<SqlParameter> parameters = null)
{
    var procedure = ParameterizeProcedure(procedureName, parameters);
    var boundParams = (parameters ?? new List<SqlParameter>()).ToArray();
    this.Database.ExecuteSqlCommand(procedure, boundParams);
}

public List<TReturn> ExecuteProcedure<TReturn>(string procedureName, List<SqlParameter> parameters = null)
    where TReturn : class
{
    var procedure = ParameterizeProcedure(procedureName, parameters);
    var boundParams = (parameters ?? new List<SqlParameter>()).ToArray();
    return this.Database.SqlQuery<TReturn>(procedure, boundParams).ToList();
}

Is this a bug or are we doing something wrong?

ejohn20 commented 5 years ago

Brett - Thanks for posting this. Based on the code above, it appears to be a bug in the analyzer. To confirm, we'd need to run this through a couple test cases and potentially adjust the rule.

Are you able to provide a sample / project / class that compiles for us to test with?