Skip to content

user: skip passwd entries with invalid IDs #267

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
sairex53 wants to merge 1 commit into from
Closed

user: skip passwd entries with invalid IDs #267

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 25 additions & 10 deletions user/user.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,13 @@ type IDMap struct {
Count int64
}

func parseLine(line []byte, v ...any) {
parseParts(bytes.Split(line, []byte(":")), v...)
func parseLine(line []byte, v ...any) error {
return parseParts(bytes.Split(line, []byte(":")), v...)
}

func parseParts(parts [][]byte, v ...any) {
func parseParts(parts [][]byte, v ...any) error {
if len(parts) == 0 {
return
return nil
}

for i, p := range parts {
Expand All @@ -78,8 +78,18 @@ func parseParts(parts [][]byte, v ...any) {
case *string:
*e = string(p)
case *int:
// "numbers", with conversion errors ignored because of some misbehaving configuration files.
*e, _ = strconv.Atoi(string(p))
// UID and GID values must fit the range supported by all platforms.
id, err := strconv.ParseInt(string(p), 10, 32)
if err != nil {
if errors.Is(err, strconv.ErrRange) {
return ErrRange
}
return fmt.Errorf("parsing integer field %d: %w", i, err)
}
if id < minID || id > maxID {
return ErrRange
}
*e = int(id)
case *int64:
*e, _ = strconv.ParseInt(string(p), 10, 64)
case *[]string:
Expand All @@ -94,6 +104,7 @@ func parseParts(parts [][]byte, v ...any) {
panic(fmt.Sprintf("parseLine only accepts {*string, *int, *int64, *[]string} as arguments! %#v is not a pointer!", e))
}
}
return nil
}

func ParsePasswdFile(path string) ([]User, error) {
Expand Down Expand Up @@ -135,7 +146,9 @@ func ParsePasswdFilter(r io.Reader, filter func(User) bool) ([]User, error) {
// root:x:0:0:root:/root:/bin/bash
// adm:x:3:4:adm:/var/adm:/bin/false
p := User{}
parseLine(line, &p.Name, &p.Pass, &p.Uid, &p.Gid, &p.Gecos, &p.Home, &p.Shell)
if err := parseLine(line, &p.Name, &p.Pass, &p.Uid, &p.Gid, &p.Gecos, &p.Home, &p.Shell); err != nil {
continue
}

if filter == nil || filter(p) {
out = append(out, p)
Expand Down Expand Up @@ -194,7 +207,9 @@ func ParseGroupFilter(r io.Reader, filter func(Group) bool) ([]Group, error) {
// root:x:0:root
// adm:x:4:root,adm,daemon
p := Group{}
parseLine(line, &p.Name, &p.Pass, &p.Gid, &p.List)
if err := parseLine(line, &p.Name, &p.Pass, &p.Gid, &p.List); err != nil {
continue
}

if filter == nil || filter(p) {
out = append(out, p)
Expand Down Expand Up @@ -501,7 +516,7 @@ func ParseSubIDFilter(r io.Reader, filter func(SubID) bool) ([]SubID, error) {

// see: man 5 subuid
p := SubID{}
parseLine(line, &p.Name, &p.SubID, &p.Count)
_ = parseLine(line, &p.Name, &p.SubID, &p.Count)

if filter == nil || filter(p) {
out = append(out, p)
Expand Down Expand Up @@ -549,7 +564,7 @@ func ParseIDMapFilter(r io.Reader, filter func(IDMap) bool) ([]IDMap, error) {

// see: man 7 user_namespaces
p := IDMap{}
parseParts(bytes.Fields(line), &p.ID, &p.ParentID, &p.Count)
_ = parseParts(bytes.Fields(line), &p.ID, &p.ParentID, &p.Count)

if filter == nil || filter(p) {
out = append(out, p)
Expand Down
61 changes: 51 additions & 10 deletions user/user_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,48 +15,53 @@ import (
)

func TestParseLine(t *testing.T) {
mustParseLine := func(line []byte, v ...any) {
t.Helper()
if err := parseLine(line, v...); err != nil {
t.Fatal(err)
}
}
var (
a, b string
c []string
d int
)

parseLine([]byte(""), &a, &b)
mustParseLine([]byte(""), &a, &b)
if a != "" || b != "" {
t.Fatalf("a and b should be empty ('%v', '%v')", a, b)
}

parseLine([]byte("a"), &a, &b)
mustParseLine([]byte("a"), &a, &b)
if a != "a" || b != "" {
t.Fatalf("a should be 'a' and b should be empty ('%v', '%v')", a, b)
}

parseLine([]byte("bad boys:corny cows"), &a, &b)
mustParseLine([]byte("bad boys:corny cows"), &a, &b)
if a != "bad boys" || b != "corny cows" {
t.Fatalf("a should be 'bad boys' and b should be 'corny cows' ('%v', '%v')", a, b)
}

parseLine([]byte(""), &c)
mustParseLine([]byte(""), &c)
if len(c) != 0 {
t.Fatalf("c should be empty (%#v)", c)
}

parseLine([]byte("d,e,f:g:h:i,j,k"), &c, &a, &b, &c)
mustParseLine([]byte("d,e,f:g:h:i,j,k"), &c, &a, &b, &c)
if a != "g" || b != "h" || len(c) != 3 || c[0] != "i" || c[1] != "j" || c[2] != "k" {
t.Fatalf("a should be 'g', b should be 'h', and c should be ['i','j','k'] ('%v', '%v', '%#v')", a, b, c)
}

parseLine([]byte("::::::::::"), &a, &b, &c)
mustParseLine([]byte("::::::::::"), &a, &b, &c)
if a != "" || b != "" || len(c) != 0 {
t.Fatalf("a, b, and c should all be empty ('%v', '%v', '%#v')", a, b, c)
}

parseLine([]byte("not a number"), &d)
if d != 0 {
t.Fatalf("d should be 0 (%v)", d)
if err := parseLine([]byte("not a number"), &d); err == nil {
t.Fatal("expected an error for an invalid numeric field")
}

parseLine([]byte("b:12:c"), &a, &d, &b)
mustParseLine([]byte("b:12:c"), &a, &d, &b)
if a != "b" || b != "c" || d != 12 {
t.Fatalf("a should be 'b' and b should be 'c', and d should be 12 ('%v', '%v', %v)", a, b, d)
}
Expand All @@ -82,6 +87,42 @@ this is just some garbage data
}
}

func TestParsePasswdFilterSkipsInvalidIDs(t *testing.T) {
valid := "valid:x:100:100::/home/valid:/bin/sh"
tests := []struct {
name string
line string
}{
{name: "invalid UID", line: "evil:x:not-a-uid:10::/home/evil:/bin/sh"},
{name: "invalid GID", line: "evil:x:10:not-a-gid::/home/evil:/bin/sh"},
{name: "UID overflow", line: "evil:x:4294967296:10::/home/evil:/bin/sh"},
{name: "negative UID", line: "evil:x:-1:10::/home/evil:/bin/sh"},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
users, err := ParsePasswdFilter(strings.NewReader(tc.line+"\n"+valid), nil)
if err != nil {
t.Fatalf("ParsePasswdFilter: %v", err)
}
if len(users) != 1 || users[0].Name != "valid" || users[0].Uid != 100 {
t.Fatalf("expected only the valid passwd entry, got %+v", users)
}
})
}
}

func TestParseGroupFilterSkipsInvalidGID(t *testing.T) {
for _, line := range []string{"evil:x:not-a-gid:root", "evil:x:4294967296:root"} {
groups, err := ParseGroupFilter(strings.NewReader(line+"\nvalid:x:42:validuser"), nil)
if err != nil {
t.Fatalf("ParseGroupFilter: %v", err)
}
if len(groups) != 1 || groups[0].Name != "valid" || groups[0].Gid != 42 {
t.Errorf("expected only the valid group entry, got %+v", groups)
}
}
}

func TestParseGroupFilter(t *testing.T) {
groups, err := ParseGroupFilter(strings.NewReader(`
root:x:0:root
Expand Down