The Wayback Machine - https://web.archive.org/web/20211003213752/https://github.com/github/codeql/pull/6777
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

Data flow: Rework SummarizedCallable::clearsContent/2 #6777

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

@hvitved
Copy link
Contributor

@hvitved hvitved commented Sep 29, 2021

This PR reworks how we interpret SummarizedCallable::clearsContent(int i, Content c). Before this PR, we would insert content-clearing at the relevant argument nodes to a summarized callable. However, that does not work when we want to model a callable that consumes some content, that is, both reads from the contents and afterwards clears it, since clearing at the argument would not allow for reading the content inside the summarized callable.

This PR instead synthesizes two new nodes inside the callable:

p --local--> synth1
 \
  --local--> synth2

where p is the ith parameter node for the callable. Content-clearing is inserted at synth1, and synth1 becomes a post-update node for synth2. This effectively allows for flow into p and back out again, but only if data is not stored in the relevant content. (We could in principle have avoided synth2 and make synth1 a post-update node for p, but that may break the invariant that each node has at most one post-update node).

The above only works, however, if flow at calls to the summarized callable does not skip over the argument. Example

a.b = taint;
a.clearB(); // assume we have a flow summary for `clearB` that clears `b` on the qualifier
sink(a.b);

In the above, flow should not pass from a on the first line (or the second line) to a on the third line. Instead, the flow summary will allow for flow from a on line 2 to the post-update node for a on that line, and from there use-use flow to a on the third line.

@hvitved hvitved force-pushed the dataflow/summary-clear-modelling branch from 43b2084 to e7059fa Sep 29, 2021
@hvitved hvitved force-pushed the dataflow/summary-clear-modelling branch from e7059fa to 6a4e2a6 Sep 30, 2021
@hvitved hvitved marked this pull request as ready for review Sep 30, 2021
@hvitved hvitved requested review from as code owners Sep 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants