Skip to content

Swift: Fix false positives in the swift/cleartext-transmission query #17210

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

Merged
merged 4 commits into from
Aug 16, 2024

Conversation

geoffw0
Copy link
Contributor

@geoffw0 geoffw0 commented Aug 13, 2024

Fix false positives in the swift/cleartext-transmission query where a tel:, mailto: or similar URL is created that contains unencrypted sensitive data (the telephone number or e-mail address) that is, strictly speaking, transmitted. However in these cases the data is only transmitted by the act of it's use, which is very much intended and unavoidable in creating that connection. This kind of result is therefore not helpful.

@geoffw0 geoffw0 added the Swift label Aug 13, 2024
@geoffw0 geoffw0 requested a review from a team as a code owner August 13, 2024 10:27
@geoffw0
Copy link
Contributor Author

geoffw0 commented Aug 13, 2024

DCA: we've lost two results, exactly the sort of thing this PR is intended to remove:

                      guard let phoneNumberUrl = URL(string: urlString) else {

(I believe urlString is "tel:" + phoneNumber)
and

          if let url = URL(string: "tel://\(callableNumber)") {

The run was a little slower (15.7%), but wobble on Swift can get that high. I'll keep an eye on the regular run after this is merged just in case. I would expect the query itself to be a bit slower due to one additional data flow being computed, but I would not expect that to meaningfully affect total analysis time.

Copy link
Contributor

@redsun82 redsun82 left a comment

Choose a reason for hiding this comment

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

Seems very reasonable.

@geoffw0 geoffw0 merged commit e3b9b0a into github:main Aug 16, 2024
21 checks passed
@geoffw0
Copy link
Contributor Author

geoffw0 commented Aug 16, 2024

Thanks!

@geoffw0 geoffw0 deleted the mailto branch October 29, 2024 16:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants