Swift: Support MaD summaries #10699
Merged
Swift: Support MaD summaries #10699
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
MathiasVP
changed the title
MathiasVP
force-pushed
the
swift-mad-summaries
branch
from
1b48c66 to
56523d4
Compare
MathiasVP
force-pushed
the
swift-mad-summaries
branch
from
56523d4 to
0065a5a
Compare
geoffw0
reviewed
Oct 6, 2022
| private class UrlSessionSummaries extends SummaryModelCsv { | ||
| override predicate row(string row) { | ||
| row = | ||
| ";URLSession;true;dataTask(with:completionHandler:);;;Argument[0];Argument[1].Parameter[0];taint" |
There was a problem hiding this comment.
Thanks for adding this, along with the test for it!
I think the data returned from a dataTask should ultimately be a taint source rather than a summary - it doesn't really matter if the URL itself was tainted, the data returned from it is. But at the very least we have something here and we know we can cope with callbacks.
There was a problem hiding this comment.
Good point. Indeed, I agree that this should be a taint source as well. Having a taint summary will still be useful though (so that we can still track flow through these library calls in queries that don't pick any RemoteFlowSource as a taint source).
There was a problem hiding this comment.
I've added it to the taint source TODO list.
Co-authored-by: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Co-authored-by: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
geoffw0
approved these changes
Oct 6, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR does a couple of MaD-summary related things:
A commit-by-commit review is encouraged. The first commit adds support for selecting fields as flow sources. This is motivated by #10597, and this PR changes those flow sources to specified using MaD.
The remaining commits adds support for specifying summaries in MaD for Swift. It took some extra dataflow work to support callbacks, but in the end, we can now track flow from
taintedUrltodata!in the call tosink: