Repository navigation
reject OEM user and group names with trailing characters - #81
Open
cameronaaron wants to merge 1 commit into
Open
cameronaaron wants to merge 1 commit into
cameronaaron wants to merge 1 commit into
Conversation
oem_id_from_name used sscanf("oem_%u"), which doesn't check that the
whole name was consumed and also accepts a sign or leading whitespace,
so names like "oem_2900x" and "oem_+2900" resolved to uid 2900. An
out-of-range value was also undefined behavior.
Require only digits after "oem_" and parse them with strtoull, rejecting
trailing characters and values that don't fit in id_t.
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.
Closes GrapheneOS/os-issue-tracker#8027
sscanf(name, "oem_%u", &id)only checks that a number was read, sogetpwnam("oem_2900x"),"oem_+2900"and"oem_ 2900"all resolved to uid 2900. This parses the id the same wayapp_id_from_namedoes (digits only, whole string consumed) and range-checks it before narrowing toid_t.Added negative cases to
getpwnam_non_existandgetgrnam_non_exist. I haven't run bionic-unit-tests on a device. I compared the new function against the old one on the host for valid and malformed names, and checked that the range comparison doesn't trigger-Wtype-limitson 32-bit targets. The same code is still in AOSP main.