libct/user: fix parsing long /etc/group lines

Lines in /etc/group longer than 64 characters breaks the current
implementation of group parser. This is caused by bufio.Scanner
buffer limit.

Fix by re-using the fix for a similar problem in golang os/user,
namely https://go-review.googlesource.com/c/go/+/283601.

Add some tests.

Co-authored-by: Andrey Bokhanko <[email protected]>
Signed-off-by: Kir Kolyshkin <[email protected]>

libcontainer/user/user.go

@@ -180,15 +180,53 @@ 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()
 
-	var (
-		s   = bufio.NewScanner(r)
-		out = []Group{}
-	)
+			if err != nil {
+				// We should return no error if EOF is reached
+				// without a match.
+				if err == io.EOF { //nolint:errorlint // comparison with io.EOF is legit, https://github.com/polyfloyd/go-errorlint/pull/12
+					err = nil
+				}
+				return out, err
+			}
 
-	for s.Scan() {
-		text := bytes.TrimSpace(s.Bytes())
-		if len(text) == 0 {
+			// Simple common case: line is short enough to fit in a
+			// single reader's buffer.
+			if !isPrefix && len(wholeLine) == 0 {
+				wholeLine = line
+				break
+			}
+
+			wholeLine = append(wholeLine, line...)
+
+			// Check if we read the whole line already.
+			if !isPrefix {
+				break
+			}
+		}
+
+		// 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] == '#' {
 			continue
 		}
 
@@ -198,17 +236,12 @@ func ParseGroupFilter(r io.Reader, filter func(Group) bool) ([]Group, error) {
 		//  root:x:0:root
 		//  adm:x:4:root,adm,daemon
 		p := Group{}
-		parseLine(text, &p.Name, &p.Pass, &p.Gid, &p.List)
+		parseLine(wholeLine, &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 {

libcontainer/user/user_test.go

@@ -1,6 +1,7 @@
 package user
 
 import (
+	"fmt"
 	"io"
 	"reflect"
 	"sort"
@@ -82,12 +83,12 @@ func TestUserParseGroup(t *testing.T) {
 root:x:0:root
 adm:x:4:root,adm,daemon
 this is just some garbage data
-`), nil)
+`+largeGroup()), nil)
 	if err != nil {
 		t.Fatalf("Unexpected error: %v", err)
 	}
-	if len(groups) != 3 {
-		t.Fatalf("Expected 3 groups, got %v", len(groups))
+	if len(groups) != 4 {
+		t.Fatalf("Expected 4 groups, got %v", len(groups))
 	}
 	if groups[0].Gid != 0 || groups[0].Name != "root" || len(groups[0].List) != 1 {
 		t.Fatalf("Expected groups[0] to be 0 - root - 1 member, got %v - %v - %v", groups[0].Gid, groups[0].Name, len(groups[0].List))
@@ -103,16 +104,18 @@ root:x:0:0:root user:/root:/bin/bash
 adm:x:42:43:adm:/var/adm:/bin/false
 111:x:222:333::/var/garbage
 odd:x:111:112::/home/odd:::::
+user7456:x:7456:100:Vasya:/home/user7456
 this is just some garbage data
 `
-	const groupContent = `
+	groupContent := `
 root:x:0:root
 adm:x:43:
-grp:x:1234:root,adm
+grp:x:1234:root,adm,user7456
 444:x:555:111
 odd:x:444:
 this is just some garbage data
-`
+` + largeGroup()
+
 	defaultExecUser := ExecUser{
 		Uid:   8888,
 		Gid:   8888,
@@ -216,6 +219,16 @@ this is just some garbage data
 				Home:  "/home/odd",
 			},
 		},
+		// Test for #3036.
+		{
+			ref: "7456",
+			expected: ExecUser{
+				Uid:   7456,
+				Gid:   100,
+				Sgids: []int{1234, 1000}, // 1000 is largegroup GID
+				Home:  "/home/user7456",
+			},
+		},
 	}
 
 	for _, test := range tests {
@@ -388,13 +401,13 @@ func TestGetAdditionalGroups(t *testing.T) {
 		hasError bool
 	}
 
-	const groupContent = `
+	groupContent := `
 root:x:0:root
 adm:x:43:
 grp:x:1234:root,adm
 adm:x:4343:root,adm-duplicate
 this is just some garbage data
-`
+` + largeGroup()
 	tests := []foo{
 		{
 			// empty group
@@ -444,6 +457,11 @@ this is just some garbage data
 			expected: nil,
 			hasError: true,
 		},
+		{
+			// group with very long list of users
+			groups:   []string{"largegroup"},
+			expected: []int{1000},
+		},
 	}
 
 	for _, test := range tests {
@@ -500,3 +518,13 @@ func TestGetAdditionalGroupsNumeric(t *testing.T) {
 		}
 	}
 }
+
+// Generate a proper "largegroup" entry for group tests.
+func largeGroup() (res string) {
+	var b strings.Builder
+	b.WriteString("largegroup:x:1000:user1")
+	for i := 2; i <= 7500; i++ {
+		fmt.Fprintf(&b, ",user%d", i)
+	}
+	return b.String()
+}