Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SEC0108 warning with recommended overload #51

Open
brettpostin opened this issue Dec 18, 2018 · 1 comment
Open

SEC0108 warning with recommended overload #51

brettpostin opened this issue Dec 18, 2018 · 1 comment

Comments

@brettpostin
Copy link

brettpostin commented Dec 18, 2018

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
Copy link
Member

ejohn20 commented Dec 19, 2018

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants