moby / moby/sys

user: TestGetAdditionalGroups/group_entry_with_out-of-range_gid fails on 32-bit architectures (GOARCH=386 / arm)

Open
#254 1 comment 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

Description

When running tests in github.com/moby/sys/user on 32-bit architectures (e.g. GOARCH=386 or GOARCH=arm), TestGetAdditionalGroups/group_entry_with_out-of-range_gid fails:

=== RUN   TestGetAdditionalGroups
=== RUN   TestGetAdditionalGroups/group_entry_with_out-of-range_gid
    user_test.go:651: Parse(struct { doc string; groups []string; expected []int; hasError bool }{doc:"group entry with out-of-range gid", groups:[]string{"toolarge"}, expected:[]int(nil), hasError:true}) expects error but has none
--- FAIL: TestGetAdditionalGroups (0.01s)
    --- FAIL: TestGetAdditionalGroups/group_entry_with_out-of-range_gid (0.00s)
FAIL	github.com/moby/sys/user	0.008s
Steps to Reproduce
cd user
GOARCH=386 go test -v . -run "TestGetAdditionalGroups/group_entry_with_out-of-range_gid"
Root Cause Analysis
  1. In user/user_test.go, the test defines groupContent with:

    toolarge:x:2147483648:
    

    where 2147483648 is math.MaxInt32 + 1.

  2. In user/user.go, parseParts() parses GID entries into *int via:

    case *int:
        *e, _ = strconv.Atoi(string(p))
    

    Conversion errors returned by strconv.Atoi are explicitly ignored (_).

  3. On 64-bit systems (int is int64), strconv.Atoi("2147483648") succeeds and sets *e = 2147483648. GetAdditionalGroups() then checks if g.Gid < minID || g.Gid > maxID (where maxID is 2147483647), correctly returning ErrRange.

  4. On 32-bit systems (int is int32), strconv.Atoi("2147483648") overflows int32 and returns strconv.ErrRange while setting *e = math.MaxInt32 (2147483647). Because the conversion error is discarded, g.Gid becomes 2147483647. Since 2147483647 <= maxID, GetAdditionalGroups() considers the GID valid, does not return an error, and the test fails.

Suggested Fix

parseParts (or Group.Gid / User.Uid / User.Gid parsing) should either:

  1. Parse IDs as int64 (e.g. strconv.ParseInt(string(p), 10, 64)) or validate strconv.ErrRange before truncating/converting to int, or
  2. Check whether the string value exceeds maxID (or check for strconv.ErrRange) when validating group/user entries so that 32-bit platforms reject out-of-range values consistently with 64-bit platforms.

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 with user/user_test.go around TestGetAdditionalGroups and user/user.go around parseParts, then run GOARCH=386 go test -v . -run "TestGetAdditionalGroups/group_entry_with_out-of-range_gid". Trace how the GID is parsed on 32-bit and 64-bit architectures. Done means the out-of-range group entry reliably returns the expected error on both architectures without regressing User or Group ID parsing.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
operating-systems
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
74/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.