TaintedDataAnalysis: Consider parameter types in ASP.NET Controller action methods

[dotpaul]

I was thinking, maybe we should look at which overload Append() is used, because we may not want, for example, integers to necessarily taint for SQL injection or anything else using the PrimitiveTypeConverterSanitizers...but nevermind. I think the better approach is to try to make not taint the integers in the first place.

Which makes me think that ASP.NET Core Controller action method parameters, should take also into the consideration of the parameter's type, for SinkKinds that are using PrimitiveTypeConvertSanitizers (SinkKinds not using PrimitiveTypeConverterSanitizers can just taint all parameters). Or do something even more fundamental and restrict which types can be tainted. I'll open an issue to ponder what to do.

Originally posted by @dotpaul in https://github.com/dotnet/roslyn-analyzers/pull/4024

[JarLob]

Sometimes tainting all arguments, not only the strings makes sense. It may catch such second order injections like:

public HttpResponseMessage GetSomething(int id)
{
    var doc = GetDocById(id);
    ReadFile(doc.Path);
}

The fact that id is user controllable may be important. Maybe this could be configurable?

Overall tainting only strings makes sense, like in the example below taint only the filter:

public HttpResponseMessage GetSomething(int id, string filter, bool whatever)

However what to do with objects like:

class User
{
    public int id;
    public string Name;
    public User TheBoss;
}
public HttpResponseMessage UpdateUser(User u)

Recursively taint all inner string properties and fields?

[dotpaul]

Sometimes tainting all arguments, not only the strings makes sense. It may catch such second order injections like:

public HttpResponseMessage GetSomething(int id)
{
    var doc = GetDocById(id);
    ReadFile(doc.Path);
}

The fact that id is user controllable may be important. Maybe this could be configurable?

True.

Overall tainting only strings makes sense, like in the example below taint only the filter:

public HttpResponseMessage GetSomething(int id, string filter, bool whatever)

However what to do with objects like:

class User
{
    public int id;
    public string Name;
    public User TheBoss;
}
public HttpResponseMessage UpdateUser(User u)

Recursively taint all inner string properties and fields?

Not that this is my preferred approach, but if looking at the parameter types of controller action methods, then I'd do something simple and only treat simple parameters (int, etc.) as untainted. The entire User object would be tainted, including its id field.

I'm leaning more towards how can TaintedDataOperationVisitor easily treat IOperations of certain types (the ones in PrimitiveTypeConverterSanitizers) as untainted. That would make u.id untainted.

TaintedDataAnalysisContext would need a set of types that would not be considered tainted.

TaintedDataConfig would need to generate the set of types that would not be considered tainted, based on the SinkKind.