C++: Add remote flow sources via models #3273
Conversation
There was a problem hiding this comment.
Otherwise LGTM. I like this design as it lets us avoid repeating the charpreds and boilerplate we already have for modelling other aspects of functions (like fgets). It's also, AFAICT, possible for users to extend RemoteFlowSource directly if they should prefer.
| * A library function which returns data read from a network connection. | ||
| */ | ||
| abstract class RemoteFlowFunction extends Function { | ||
| abstract predicate hasFlowSource(FunctionOutput output); |
There was a problem hiding this comment.
If we later add a LocalFlowFunction, like C# has LocalFlowSource, the name hasFlowSource becomes ambiguous. I suggest renaming to hasRemoteFlowSource.
| * A library function which returns data read from a network connection. | ||
| */ | ||
| abstract class RemoteFlowFunction extends Function { | ||
| abstract predicate hasFlowSource(FunctionOutput output); |
There was a problem hiding this comment.
Would it be any problem to add a string parameter to hold the textual description of the flow type, like you did in #3274 to be compatible with the other languages?
Co-Authored-By: Jonas Jensen <jbj@github.com>
rdmarsh2
force-pushed
the
rdmarsh/cpp/RemoteFlowSource-model
branch
from
f3b84fe to
d0bb5ad
Compare
| output.isParameterDeref(0) | ||
| override predicate hasRemoteFlowSource(FunctionOutput output, string description) { | ||
| output.isParameterDeref(0) and | ||
| description = "String read by " + this.getName() |
There was a problem hiding this comment.
This description looks good enough to me. If we're not planning to provide any more detail, then it doesn't seem necessary to have a description parameter on hasRemoteFlowSource. The same information is available in the Tainted{Parameter,Return}Source classes, and it should be less work to construct the description there than to repeat it for every RemoteFlowFunction.
There was a problem hiding this comment.
I was expecting that we would provide more detail when it's useful, but fread and gets are the least-specific sources we'll deal with.
This adds a new
RemoteFlowSourceclass for IR flow, based on a new model interface. I've included flow source models forfreadandgets, since they had model classes already.This is consistent with the direction the C++ library is moving for flow sinks such as allocation sizes and format arguments, but rather different from the Java and C# versions of
RemoteFlowSource. I've also opened #3274 showing a version that's closer to the Java and C# versions.