user: parseLine silently ignores uid/gid conversion errors, malformed ids parse as 0
Nobody has claimed this yet.
- Dominant language
- Go
- Stars
- 101
- Forks
- 61
- Avg merge
- 2d 7h
- Merged PRs (30d)
- 5
Description
parseParts in user/user.go parses numeric fields with the conversion error discarded:
case *int:
// "numbers", with conversion errors ignored because of some misbehaving configuration files.
*e, _ = strconv.Atoi(string(p))
https://github.com/moby/sys/blob/d6b812df97ee7e0c1e16437fd4d1fa61738427db/user/user.go#L82
Two consequences for consumers that resolve container users from an image's /etc/passwd or /etc/group:
- A malformed id field parses as 0. The line
evil:x:not-a-uid:10::/:/bin/shproducesUser{Name: "evil", Uid: 0}, so looking up that user silently yields root. The caller can't detect this, since uid 0 is also a perfectly legitimate value. - An id past the int range saturates instead of erroring. On 32-bit platforms
4294967296comes back as 2147483647 with no error, so the caller can't range-check it either.
Context: containerd/containerd#13797 added bounds checks on ids resolved from image user databases, but both cases above are invisible to that check because the value has already been collapsed by the time ParsePasswdFilter/ParseGroupFilter return. @fuweid suggested fixing this at the parser level (https://github.com/containerd/containerd/pull/13797#discussion_r3867287873).
I understand the error was dropped deliberately to tolerate misbehaving config files, but for the id fields specifically the failure mode is "arbitrary string becomes root", which seems worth surfacing. Happy to send a PR, either returning an error from the parse functions when an id field doesn't parse, or skipping such entries, whichever direction you prefer.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in user/user.go at parseParts and trace how ParsePasswdFilter and ParseGroupFilter expose parsed IDs. Reproduce the malformed and out-of-range examples from the issue, then determine whether the chosen behavior is to return conversion errors or skip invalid entries. Done means callers can distinguish invalid IDs from legitimate uid 0 values.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- backend, security
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 68/100