Sitelet https://web.archive.org/web/20260322181222/https://github.com/github/codeql/pull/9972
Skip to content

Swift: Taint through interpolated strings#9972

Merged
MathiasVP merged 9 commits intogithub:mainfrom
MathiasVP:swift-taint-through-interpolated-strings
Aug 5, 2022
Merged

Swift: Taint through interpolated strings#9972
MathiasVP merged 9 commits intogithub:mainfrom
MathiasVP:swift-taint-through-interpolated-strings

Conversation

@MathiasVP
Copy link
Contributor

@MathiasVP MathiasVP commented Aug 4, 2022 •

This PR adds control-flow and taint-flow through interpolated string literals in Swift.

Note: Don't merge this before we've merged #9964 as it (semantically) conflicts with that PR. That PR has been merged now 🎉

Commit-by-commit review strongly encouraged!

@MathiasVP MathiasVP requested a review from rdmarsh2 August 4, 2022 19:12
@MathiasVP MathiasVP requested a review from a team as a code owner August 4, 2022 19:12
@github-actions github-actions bot added the Swift label Aug 4, 2022
@MathiasVP MathiasVP force-pushed the swift-taint-through-interpolated-strings branch from 54d7a3f to 74dd20a Compare August 4, 2022 19:14
@MathiasVP MathiasVP force-pushed the swift-taint-through-interpolated-strings branch from 74dd20a to 05e6dd8 Compare August 4, 2022 20:57
Copy link
Contributor

@geoffw0 geoffw0 left a comment

Choose a reason for hiding this comment

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

Results in the tests look good, but I'm struggling to give this any deeper review at the moment.


webview.loadHTMLString("<html>\(localStringFragment)</html>", baseURL: nil) // GOOD: the HTML data is local
webview.loadHTMLString("<html>\(remoteString)</html>", baseURL: nil) // BAD [NOT DETECTED]
webview.loadHTMLString("<html>\(remoteString)</html>", baseURL: nil) // BAD
Copy link
Contributor

Choose a reason for hiding this comment

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

Fantastic!

Copy link
Contributor

@geoffw0 geoffw0 left a comment

Choose a reason for hiding this comment

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

I'm happy with this, I think, but I'd like to give others a little longer to comment.

@MathiasVP MathiasVP merged commit f2767eb into github:main Aug 5, 2022
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.

4 participants