Skip to content

Swift: Models and tests for numeric conversions #13946

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 21 commits into from
Sep 22, 2023
Merged

Swift: Models and tests for numeric conversions #13946

merged 21 commits into from
Sep 22, 2023

Conversation

geoffw0
Copy link
Contributor

@geoffw0 geoffw0 commented Aug 10, 2023 •
edited
Loading

Add models and tests for (mostly) numeric conversions.

Fixed a flow issue with OptionalSomePattern I bumped into on the way.

TODO:

  • can we catch this one as well:
	sink(arg: [UInt8](sourceString().utf8)) // $ MISSING: tainted=
  • DCA run

@geoffw0 geoffw0 added the Swift label Aug 10, 2023
@geoffw0 geoffw0 requested a review from a team as a code owner August 10, 2023 18:56
@geoffw0 geoffw0 mentioned this pull request Aug 10, 2023
Draft
@geoffw0
Copy link
Contributor Author

geoffw0 commented Aug 31, 2023

DCA LGTM. I will fix the merge conflicts after this has had an initial review.

d10c
d10c reviewed Sep 18, 2023
View reviewed changes
Copy link
Contributor

@d10c d10c left a comment

Choose a reason for hiding this comment

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

LGTM

@geoffw0
Copy link
Contributor Author

geoffw0 commented Sep 19, 2023

Fixed merge conflicts. Changes after the merge need a quick review.

There were a number of changes to various test results as a result of the merge. One was a good change, the other two were undesirable. I've addressed them by adding proper barriers for Numeric values (in particular Int) in five injection-like queries. Previously we were depending on data flow not actually working through the constructs that were involved.

@geoffw0
Copy link
Contributor Author

geoffw0 commented Sep 19, 2023

... I should probably add a second change note for the barriers ...

@geoffw0
Copy link
Contributor Author

geoffw0 commented Sep 19, 2023

Second change note added (with no mention of the command injection query as it's still in experimental).

And added the numeric barrier to the regular expression injection query as well.

d10c
d10c approved these changes Sep 22, 2023
View reviewed changes
Copy link
Contributor

@d10c d10c left a comment

Choose a reason for hiding this comment

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

Looks good :shipit:

@geoffw0 geoffw0 merged commit 2c9433c into github:main Sep 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@d10c d10c d10c approved these changes

Assignees
No one assigned
Labels
documentation Swift
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@geoffw0 @d10c