Repository navigation
process: compare argv[0] and AT_EXECFN basenames as bytes - #404
Open
pierre-warnier wants to merge 1 commit into
Open
pierre-warnier wants to merge 1 commit into
pierre-warnier wants to merge 1 commit into
Conversation
The setuid check converted both basenames with to_string_lossy before comparing them. That maps every invalid UTF-8 sequence to U+FFFD, so two different names such as "pw\xfe" and "pw\xff" compared equal. Compare the bytes instead, and treat a path with no basename as matching nothing. The multicall dispatch matched utility names against lossily converted strings too. Utility names are UTF-8, so take the name only when it is valid UTF-8; anything else names no utility, as before. Suggested-by: oech3
sylvestre
reviewed
Nov 8, 2026
| // Direct invocation via symlink (e.g., argv[0] = "passwd") | ||
| if let Some(run) = find_applet(&binary_name) { | ||
| return run_applet(&binary_name, run, &args); | ||
| if let Some((name, run)) = binary_name.and_then(|n| find_applet(n).map(|run| (n, run))) { |
Collaborator
There was a problem hiding this comment.
a bit convoluted, could be if let Some(name) = binary_name && let Some(run) = find_applet(name), no?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
@oech3 suggested in #402 (comment) that the argv[0] check avoid lossy conversion. This does that.
verify_argv0_matches_execfnran both basenames throughto_string_lossybefore comparing them. Lossy conversion maps every invalid UTF-8 sequence to U+FFFD, so two different names compared equal:pw\xfeagainst anAT_EXECFNof/tmp/pw\xff, or a literal U+FFFD against\xff.That was not exploitable. A lossy string equals an ASCII utility name only when the original bytes were exactly that name, so a collision could only reach the multicall path, and a caller can reach that path anyway by running the binary under its real name. It was still a check that said "equal" for different names.
Changes:
verify_argv0_matches_execfntakes&OsStrand compares the basenames byte for byte. A path with no basename, such as an emptyargv[0], matches nothing. Before, two missing basenames compared equal as empty strings. The null check ongetauxvalstays, and theunsafeblock is unchanged.argv[0]to the check. When it looks up utility names, inargv[0]or in the first argument, it takes the name only if it is valid UTF-8. Utility names are UTF-8, so anything else names no utility, as before. The "unknown utility" message still shows the argument lossily, since it is only displayed.Tests: unit tests for the basename comparison (different invalid bytes, U+FFFD against
\xff, identical invalid bytes, missing basenames). Two of them fail with the old lossy comparison. There is also a multicall test showing that a non-UTF-8argv[0]or applet argument is refused.Reviews welcome. Without objection, this will be merged after 7 days.