Skip to content

[Demangler] Accept overly short type names if they are NUL terminated #77346

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 1 commit into from
Nov 5, 2024

Conversation

mikeash
Copy link
Contributor

@mikeash mikeash commented Nov 1, 2024

swift_getTypeByMangledNameInContext takes a pointer and a length, but some programs pass a pointer to a NUL-terminated C string and an excessive length, implicitly relying on the terminator to end the string early. This worked previously, but commit 7fe2bef made the demangler more strict about bad data. This changes the demangler to successfully parse a name by terminating the name string at a 0 byte encountered where it expects to find an operator. All other cases of bad data continue to be rejected.

rdar://137430048

@mikeash mikeash requested review from al45tair and eeckstein November 1, 2024 16:06
@mikeash mikeash requested a review from rjmccall as a code owner November 1, 2024 16:07
Copy link
Contributor

@al45tair al45tair left a comment

Choose a reason for hiding this comment

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

Yuck. Manglings can contain embedded NUL characters in some circumstances, so this is pretty horrible. I don't see an alternative, though, and it's allowing NUL only when we're expecting an operator so maybe that's OK.

Copy link
Contributor

@eeckstein eeckstein left a comment

Choose a reason for hiding this comment

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

Looks like a reasonable compromise

swift_getTypeByMangledNameInContext takes a pointer and a length, but some programs pass a pointer to a NUL-terminated C string and an excessive length, implicitly relying on the terminator to end the string early. This worked previously, but commit 7fe2bef made the demangler more strict about bad data. This changes the demangler to successfully parse a name by terminating the name string at a 0 byte encountered where it expects to find an operator. All other cases of bad data continue to be rejected.

rdar://137430048
@mikeash mikeash force-pushed the demangler-nul-terminator branch from 65153ad to 4a5dc2a Compare November 4, 2024 16:22
@mikeash
Copy link
Contributor Author

mikeash commented Nov 4, 2024

Yes, this still properly handles the expected embedded NULs, we just accept names where a NUL occurs where we expect an operator, rather than rejecting them.

I've also added a test case so we can ensure this keeps working.

@mikeash
Copy link
Contributor Author

mikeash commented Nov 4, 2024

@swift-ci please test

@mikeash mikeash enabled auto-merge November 4, 2024 17:17
@mikeash
Copy link
Contributor Author

mikeash commented Nov 4, 2024

@swift-ci please test

@mikeash
Copy link
Contributor Author

mikeash commented Nov 4, 2024

Hit the failure fixed by #77366

@mikeash
Copy link
Contributor Author

mikeash commented Nov 4, 2024

@swift-ci please smoke test

@mikeash mikeash merged commit 794165a into swiftlang:main Nov 5, 2024
3 of 5 checks passed
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.

3 participants