Skip to content
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

Mitigate misuse of Swift's pointer conversion feature #80

Merged
merged 4 commits into from
Jun 3, 2022

Conversation

glessard
Copy link
Contributor

@glessard glessard commented Apr 8, 2022

This is a continuation of the work done in apple/swift#42002, addressing the exact same issues.

The family of String (and FilePath) initializers that convert from C strings (null-terminated byte buffers) can be called with Swift arrays, which are converted to UnsafePointer arguments for C interoperability. However, when the array passed in to them violates the C string precondition of containing a zero byte, this can result in a buffer overflow.

This PR overloads every such initializer with a version for [CodeUnit] and inout CodeUnit, enforcing the null-terminated precondition. An overload for String is also added. The String overload may appear strictly useless, but it behaves differently than a direct copy when the source string contains an embedded null.

Addresses rdar://91436410.

@glessard glessard requested a review from milseman April 8, 2022 12:46
Copy link
Contributor

@milseman milseman left a comment

Choose a reason for hiding this comment

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

Put doc comments and some cleanup, otherwise LGTM

Sources/System/FilePath/FilePathString.swift Show resolved Hide resolved
Sources/System/FilePath/FilePathString.swift Show resolved Hide resolved
Sources/System/FilePath/FilePathString.swift Outdated Show resolved Hide resolved
Sources/System/FilePath/FilePathString.swift Outdated Show resolved Hide resolved
@glessard glessard force-pushed the glessard/rdar91436410 branch 2 times, most recently from 6a3f211 to d8b64ad Compare May 11, 2022 08:56
@glessard
Copy link
Contributor Author

@swift-ci please test

@glessard
Copy link
Contributor Author

@swift-ci please test

@glessard
Copy link
Contributor Author

@swift-ci please test

Copy link
Member

@lorentey lorentey left a comment

Choose a reason for hiding this comment

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

Shame we need to add all these overloads -- can't wait to have a language-level solution.

@glessard
Copy link
Contributor Author

glessard commented Jun 3, 2022

@swift-ci please test

@glessard glessard merged commit bdb13a5 into main Jun 3, 2022
@glessard glessard deleted the glessard/rdar91436410 branch June 3, 2022 22:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants