Swift: Taint flow improvements #10000
Conversation
swift/ql/lib/codeql/swift/dataflow/internal/TaintTrackingPrivate.qll
Outdated
Show resolved
Hide resolved
I'm not surprised MaD summaries for taint isn't working. I've only added support for MaD sources so far. It shouldn't be difficult to add MaD support for flow steps, though - that work just hasn't been scheduled yet. If we're starting on library modeling we should schedule it soon, though. |
swift/ql/lib/codeql/swift/dataflow/internal/TaintTrackingPrivate.qll
Outdated
Show resolved
Hide resolved
| let dataTainted2 = Data(dataTainted) | ||
|
|
||
| sink(arg: dataClean) | ||
| sink(arg: dataTainted) // tainted [NOT DETECTED] |
There was a problem hiding this comment.
Now that we have InlineExpectationsTest for Swift we should convert out tests to use that framework. No need to do it in this PR, though.
There was a problem hiding this comment.
Yeah, that might be good as a follow-up PR.
There was a problem hiding this comment.
Added to the issue.
There was a problem hiding this comment.
I can make an inline expectation test for the sinks that taint reaches (annotated with where taint came from), but it won't have the same detail (path edges etc) as the query as it is. Do you think I should keep both queries or prefer just one?
There was a problem hiding this comment.
Yeah, I think there's value in testing the paths as well.
There was a problem hiding this comment.
I'm a little reluctant to have three similar but different tests on the same code (when you include LocalTaint.ql), but the taint tests are really important. 👍
Do you know what needs doing for this? I had a look but couldn't see anything obviously missing (apart from a TODO about return types). |
We have the dataflow node that represents flow through MaD. But that node is not currently being propagated anywhere (i.e., we need to extend the set of |
swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll
Outdated
Show resolved
Hide resolved
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Taint flow improvements for Swift:
UnsafeWebViewFetch.qltoTaintTrackingPrivate.qllso that all queries can benefit.The next step (in a follow-up PR) will be converting these to models-as-data summaries, which are more concise (but my first attempt isn't working). With that I should be able to add the required bits to cover most / all of the rest of the test cases quite easily.