Skip to content

C++: IR dataflow through modeled functions#2703

Merged
jbj merged 6 commits intogithub:masterfrom
rdmarsh2:connect-ir-dataflow-models
Feb 5, 2020
Merged

C++: IR dataflow through modeled functions#2703
jbj merged 6 commits intogithub:masterfrom
rdmarsh2:connect-ir-dataflow-models

Conversation

@rdmarsh2
Copy link
Contributor

No description provided.

@rdmarsh2 rdmarsh2 added the C++ label Jan 28, 2020
@rdmarsh2 rdmarsh2 requested a review from jbj January 28, 2020 00:49
@rdmarsh2 rdmarsh2 requested a review from a team as a code owner January 28, 2020 00:49
@jbj
Copy link
Contributor

jbj commented Jan 29, 2020

Does DefaultTaintTracking benefit from this?

@jbj
Copy link
Contributor

jbj commented Jan 29, 2020

These changes that you're copying from DefaultTaintTracking to DataFlow are full of conflation between pointer and object. Wouldn't it be best to keep that conflation in the taint-tracking libraries?

@rdmarsh2
Copy link
Contributor Author

Does DefaultTaintTracking benefit from this?

It should, since it doesn't use DataFlowFunction, only TaintFunction. I'm not sure if any of the existing tests will be affected by it (they weren't when I first submitted it, but newer PRs might have changed that).

These changes that you're copying from DefaultTaintTracking to DataFlow are full of conflation between pointer and object. Wouldn't it be best to keep that conflation in the taint-tracking libraries?

Yes. I'll rewrite to remove that conflation where we have side effect instructions for both (return values and qualifiers don't currently have side effect instructions.)

Comment on lines 355 to 369
private predicate modelFlowToParameter(Function f, int parameterIn, int parameterOut) {
exists(FunctionInput modelIn, FunctionOutput modelOut |
f.(DataFlowFunction).hasDataFlow(modelIn, modelOut) and
(modelIn.isParameter(parameterIn) or modelIn.isParameterDeref(parameterIn)) and
modelOut.isParameterDeref(parameterOut)
)
}

private predicate modelFlowToReturnValue(Function f, int parameterIn) {
// Data flow from parameter to return value
exists(FunctionInput modelIn, FunctionOutput modelOut |
f.(DataFlowFunction).hasDataFlow(modelIn, modelOut) and
(modelIn.isParameter(parameterIn) or modelIn.isParameterDeref(parameterIn)) and
(modelOut.isReturnValue() or modelOut.isReturnValueDeref())
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't find any callers of these two private predicates

exists(int indexIn |
modelTaintToReturnValue(call.getStaticCallTarget(), indexIn) and
i1 = getACallArgumentOrIndirection(call, indexIn)
i1 = DataFlow::getACallArgumentOrIndirection(call, indexIn)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like there's no longer a need to copy this predicate to DataFlowUtil.qll after your latest changes.

@jbj jbj merged commit 2928f9e into github:master Feb 5, 2020
@rdmarsh2 rdmarsh2 deleted the connect-ir-dataflow-models branch February 5, 2020 19:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants