moby / moby/sys

user: parseLine silently ignores uid/gid conversion errors, malformed ids parse as 0

Open
#248 0 comments 0 reactions 0 assignees View on GitHub

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/sh produces User{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 4294967296 comes 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

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.