Merge commit from fork

Merge fixes

Author: thaJeztah

user/user.go

@@ -157,7 +157,7 @@ func ParseGroup(group io.Reader) ([]Group, error) {
 }
 
 func ParseGroupFileFilter(path string, filter func(Group) bool) ([]Group, error) {
-	group, err := os.Open(path)
+	group, err := openUserFile(path)
 	if err != nil {
 		return nil, err
 	}
@@ -169,52 +169,22 @@ func ParseGroupFilter(r io.Reader, filter func(Group) bool) ([]Group, error) {
 	if r == nil {
 		return nil, errors.New("nil source for group-formatted data")
 	}
-	rd := bufio.NewReader(r)
-	out := []Group{}
-
-	// Read the file line-by-line.
-	for {
-		var (
-			isPrefix  bool
-			wholeLine []byte
-			err       error
-		)
-
-		// Read the next line. We do so in chunks (as much as reader's
-		// buffer is able to keep), check if we read enough columns
-		// already on each step and store final result in wholeLine.
-		for {
-			var line []byte
-			line, isPrefix, err = rd.ReadLine()
-			if err != nil {
-				// We should return no error if EOF is reached
-				// without a match.
-				if err == io.EOF {
-					err = nil
-				}
-				return out, err
-			}
 
-			// Simple common case: line is short enough to fit in a
-			// single reader's buffer.
-			if !isPrefix && len(wholeLine) == 0 {
-				wholeLine = line
-				break
-			}
+	var (
+		s   = bufio.NewScanner(r)
+		out = []Group{}
+	)
 
-			wholeLine = append(wholeLine, line...)
-
-			// Check if we read the whole line already.
-			if !isPrefix {
-				break
-			}
-		}
+	// A group's user_list may be arbitrarily long, so allow lines that are
+	// much larger than bufio.Scanner's default maximum token size (64 KiB).
+	s.Buffer(nil, 1024*1024)
 
+	for s.Scan() {
 		// There's no spec for /etc/passwd or /etc/group, but we try to follow
 		// the same rules as the glibc parser, which allows comments and blank
 		// space at the beginning of a line.
-		wholeLine = bytes.TrimSpace(wholeLine)
-		if len(wholeLine) == 0 || wholeLine[0] == '#' {
+		line := bytes.TrimSpace(s.Bytes())
+		if len(line) == 0 || line[0] == '#' {
 			continue
 		}
 
@@ -224,12 +194,17 @@ func ParseGroupFilter(r io.Reader, filter func(Group) bool) ([]Group, error) {
 		//  root:x:0:root
 		//  adm:x:4:root,adm,daemon
 		p := Group{}
-		parseLine(wholeLine, &p.Name, &p.Pass, &p.Gid, &p.List)
+		parseLine(line, &p.Name, &p.Pass, &p.Gid, &p.List)
 
 		if filter == nil || filter(p) {
 			out = append(out, p)
 		}
 	}
+	if err := s.Err(); err != nil {
+		return nil, err
+	}
+
+	return out, nil
 }
 
 type ExecUser struct {
@@ -246,12 +221,12 @@ type ExecUser struct {
 func GetExecUserPath(userSpec string, defaults *ExecUser, passwdPath, groupPath string) (*ExecUser, error) {
 	var passwd, group io.Reader
 
-	if passwdFile, err := os.Open(passwdPath); err == nil {
+	if passwdFile, err := openUserFile(passwdPath); err == nil {
 		passwd = passwdFile
 		defer passwdFile.Close()
 	}
 
-	if groupFile, err := os.Open(groupPath); err == nil {
+	if groupFile, err := openUserFile(groupPath); err == nil {
 		group = groupFile
 		defer groupFile.Close()
 	}

user/user_test.go

@@ -1,9 +1,13 @@
 package user
 
 import (
+	"bytes"
 	"fmt"
 	"io"
+	"os"
+	"path/filepath"
 	"reflect"
+	"slices"
 	"sort"
 	"strconv"
 	"strings"
@@ -98,6 +102,91 @@ this is just some garbage data
 	}
 }
 
+// TestParseGroupFileCapsReads asserts the boundary behavior of the read cap:
+// well below, ending exactly at, and past maxUserFileBytes.
+func TestParseGroupFileCapsReads(t *testing.T) {
+	beyond := []byte("\nbeyond:x:42:\n")
+
+	for _, tc := range []struct {
+		name     string
+		padBytes int
+		expGIDs  []int
+		expErr   string
+	}{
+		{
+			name:     "pad below cap, beyond is parsed",
+			padBytes: 100,
+			expGIDs:  []int{42},
+		},
+		{
+			name:     "beyond ends exactly at cap, is parsed",
+			padBytes: maxUserFileBytes - len(beyond),
+			expGIDs:  []int{42},
+		},
+		{
+			name:     "pad past cap, read errors out",
+			padBytes: maxUserFileBytes,
+			expErr:   "file exceeds",
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			tmpDir := t.TempDir()
+			fileName := filepath.Join(tmpDir, "etc-group")
+
+			data := append(bytes.Repeat([]byte{0}, tc.padBytes), beyond...)
+			err := os.WriteFile(fileName, data, 0o644)
+			if err != nil {
+				t.Fatal(err)
+			}
+			gids, err := ParseGroupFile(fileName)
+			if tc.expErr != "" {
+				if err == nil {
+					t.Fatal("expected error")
+				}
+				if !strings.Contains(err.Error(), tc.expErr) {
+					t.Fatalf("unexpected error: %s", err)
+				}
+				return
+			}
+			if err != nil {
+				t.Fatal(err)
+			}
+			haveGids := make([]int, 0, len(gids))
+			for _, g := range gids {
+				haveGids = append(haveGids, g.Gid)
+			}
+			if !slices.Equal(haveGids, tc.expGIDs) {
+				t.Fatalf("unexpected gids: got %v, want %v", gids, tc.expGIDs)
+			}
+		})
+	}
+}
+
+// TestTestParseGroupFileCapsReadsonRegularFile verifies that non-regular files
+// are refused.
+func TestTestParseGroupFileCapsReadsonRegularFile(t *testing.T) {
+	fileName := t.TempDir()
+	_, err := ParseGroupFile(fileName)
+	if err == nil {
+		t.Fatal("expected error")
+	}
+	if !strings.Contains(err.Error(), "not a regular file") {
+		t.Fatalf("unexpected error: %s", err)
+	}
+}
+
+func TestParseGroupFilterDevZero(t *testing.T) {
+	dn, err := os.Open("/dev/zero")
+	if err != nil {
+		t.Fatal(err)
+	}
+	const expErr = "bufio.Scanner: token too long"
+	_, err = ParseGroupFilter(dn, nil)
+	if err == nil || !strings.Contains(err.Error(), expErr) {
+		t.Fatalf("want %q, got %v", expErr, err)
+	}
+}
+
 func TestGetExecUser(t *testing.T) {
 	const passwdContent = `
 root:x:0:0:root user:/root:/bin/bash

user/user_utils.go

@@ -0,0 +1,64 @@
+package user
+
+import (
+	"errors"
+	"fmt"
+	"io"
+	"os"
+)
+
+// maxUserFileBytes caps how much data is read from any user-database file.
+// User database files are expected to be relatively small. 10 MiB provides
+// generous headroom while bounding memory usage.
+const maxUserFileBytes = 10 << 20
+
+// openUserFile attempts to open a user-database file with a limitedFile
+// capped at maxUserFileBytes. It produces an error if the given path is
+// a non-regular file.
+func openUserFile(path string) (*limitedFile, error) {
+	f, err := os.Open(path)
+	if err != nil {
+		return nil, err
+	}
+
+	info, err := f.Stat()
+	if err != nil {
+		_ = f.Close()
+		return nil, err
+	}
+	if !info.Mode().IsRegular() {
+		_ = f.Close()
+		return nil, &os.PathError{
+			Op:   "open",
+			Path: path,
+			Err:  errors.New("not a regular file"),
+		}
+	}
+
+	return &limitedFile{
+		File: f,
+		// Allow one byte past the cap so an overflow surfaces as an
+		// error rather than a silent EOF that the parser would treat as
+		// a clean end-of-file (and miss any entries past the cap).
+		LimitedReader: &io.LimitedReader{R: f, N: maxUserFileBytes + 1},
+		name:          path,
+	}, nil
+}
+
+type limitedFile struct {
+	*os.File
+	*io.LimitedReader
+	name string
+}
+
+func (l *limitedFile) Read(p []byte) (int, error) {
+	n, err := l.LimitedReader.Read(p)
+	if l.LimitedReader.N == 0 {
+		return n, &os.PathError{
+			Op:   "read",
+			Path: l.name,
+			Err:  fmt.Errorf("file exceeds %d bytes", maxUserFileBytes),
+		}
+	}
+	return n, err
+}