fix(executor): check archive links against the stripped root and cap the decompressed stream

commit: 9f6323c18f0ccce07a3cf79710d7ca81841acb12
author: brice · 8d ago · 2026-10-14 02:43 UTC
parent: 8111cdb
signature: GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email

3 files · +191 -49

M internal/executor/actions_download.go +5 -16


@@ -171,32 +171,21 @@ func (j *job) downloadAction(ctx context.Context, num int, ra ResolvedAction, na
171	171		if c := j.cfg.Credential; c != nil {
172	172		owner = &[2]int{int(c.Uid), int(c.Gid)}
173	173		}
174		-	err = extractTarball(af, staging, owner)
	174	+	// The top directory is stripped while extracting, so that links are checked
	175	+	// against the tree they will live in (see extractTarball).
	176	+	err = extractTarball(ctx, af, staging, owner)
175	177		_ = af.Close()
176	178		if err != nil {
177	179		return ghErrorf("Can't extract the archive of action %s@%s: %v", name, ref, err)
178	180		}
179		-	ents, err := os.ReadDir(staging)
180		-	if err != nil {
181		-	return err
182		-	}
183		-	var dirs []os.DirEntry
184		-	for _, e := range ents {
185		-	if e.IsDir() {
186		-	dirs = append(dirs, e)
187		-	}
188		-	}
189		-	if len(dirs) != 1 \|\| len(ents) != 1 {
190		-	return fmt.Errorf("'%s' contains '%d' directories", archive, len(dirs))
191		-	}
192	181		_ = os.RemoveAll(dir)
193	182		if err := os.MkdirAll(filepath.Dir(dir), 0o755); err != nil {
194	183		return err
195	184		}
196		-	if err := os.Rename(filepath.Join(staging, dirs[0].Name()), dir); err != nil {
	185	+	if err := os.Rename(staging, dir); err != nil {
197	186		return err
198	187		}
199		-	j.debug(num, fmt.Sprintf("Unwrap '%s' to '%s'", dirs[0].Name(), dir))
	188	+	j.debug(num, fmt.Sprintf("Unwrap the archive to '%s'", dir))
200	189		if err := os.WriteFile(watermark(dir), []byte(j.now().UTC().Format(time.DateTime)), 0o644); err != nil {
201	190		return err
202	191		}

 	if c := j.cfg.Credential; c != nil {
 		owner = &[2]int{int(c.Uid), int(c.Gid)}
 	}
-	err = extractTarball(af, staging, owner)
+	// The top directory is stripped while extracting, so that links are checked
+	// against the tree they will live in (see extractTarball).
+	err = extractTarball(ctx, af, staging, owner)
 	_ = af.Close()
 	if err != nil {
 		return ghErrorf("Can't extract the archive of action %s@%s: %v", name, ref, err)
 	}
-	ents, err := os.ReadDir(staging)
-	if err != nil {
-		return err
-	}
-	var dirs []os.DirEntry
-	for _, e := range ents {
-		if e.IsDir() {
-			dirs = append(dirs, e)
-		}
-	}
-	if len(dirs) != 1 || len(ents) != 1 {
-		return fmt.Errorf("'%s' contains '%d' directories", archive, len(dirs))
-	}
 	_ = os.RemoveAll(dir)
 	if err := os.MkdirAll(filepath.Dir(dir), 0o755); err != nil {
 		return err
 	}
-	if err := os.Rename(filepath.Join(staging, dirs[0].Name()), dir); err != nil {
+	if err := os.Rename(staging, dir); err != nil {
 		return err
 	}
-	j.debug(num, fmt.Sprintf("Unwrap '%s' to '%s'", dirs[0].Name(), dir))
+	j.debug(num, fmt.Sprintf("Unwrap the archive to '%s'", dir))
 	if err := os.WriteFile(watermark(dir), []byte(j.now().UTC().Format(time.DateTime)), 0o644); err != nil {
 		return err
 	}

M internal/executor/actions_extract.go +78 -15


@@ -5,6 +5,7 @@ package executor
5	5		import (
6	6		"archive/tar"
7	7		"compress/gzip"
	8	+	"context"
8	9		"errors"
9	10		"fmt"
10	11		"io"

@@ -14,10 +15,14 @@ import (
14	15		)
15	16
16	17		// Limits of an action archive. The runner has none (it runs tar); these keep a
17		-	// hostile archive from filling the sandbox.
18		-	const (
	18	+	// hostile archive from filling the sandbox. They are variables for the tests.
	19	+	var (
19	20		maxArchiveEntries = 200_000
20		-	maxArchiveBytes = 2 << 30 // unpacked
	21	+	// maxArchiveBytes bounds the sizes of the regular files; maxStreamBytes the
	22	+	// whole decompressed tar stream, headers and PAX/GNU long-name records
	23	+	// included, which the entry and size counters do not see.
	24	+	maxArchiveBytes = int64(2 << 30)
	25	+	maxStreamBytes = maxArchiveBytes + 64<<20
21	26		)
22	27
23	28		// errArchive is returned for an archive that is not safe or not valid. Its text

@@ -27,12 +32,34 @@ type errArchive string
27	32
28	33		func (e errArchive) Error() string { return string(e) }
29	34
	35	+	// capReader fails once more than n bytes were read.
	36	+	type capReader struct {
	37	+	r io.Reader
	38	+	n int64
	39	+	}
	40	+
	41	+	func (c *capReader) Read(p []byte) (int, error) {
	42	+	if c.n <= 0 {
	43	+	return 0, errArchive("the archive is too large once unpacked")
	44	+	}
	45	+	if int64(len(p)) > c.n {
	46	+	p = p[:c.n]
	47	+	}
	48	+	n, err := c.r.Read(p)
	49	+	c.n -= int64(n)
	50	+	return n, err
	51	+	}
	52	+
30	53		// extractTarball unpacks a gzip-compressed tar into dir, which must exist and
31		-	// be empty. Every name is checked: no absolute path, no "..", no link whose
32		-	// target leaves dir, and everything is written through an os.Root, so that a
33		-	// link planted by an earlier entry cannot redirect a later one. owner, when
34		-	// set, is the uid and gid of the files.
35		-	func extractTarball(r io.Reader, dir string, owner *[2]int) error {
	54	+	// be empty, and strips the single top directory of the archive (GitHub's
	55	+	// archives have exactly one, ActionManager.cs:1380-1393): the content lands in
	56	+	// dir itself, so no file moves afterwards and a link that is valid here stays
	57	+	// valid. Every name is checked: no absolute path, no "..", one top directory,
	58	+	// no link whose target leaves dir, and everything is written through an
	59	+	// os.Root, so that a link planted by an earlier entry cannot redirect a later
	60	+	// one. The decompressed stream is capped, and ctx is checked between entries.
	61	+	// owner, when set, is the uid and gid of the files.
	62	+	func extractTarball(ctx context.Context, r io.Reader, dir string, owner *[2]int) error {
36	63		zr, err := gzip.NewReader(r)
37	64		if err != nil {
38	65		return errArchive("the archive is not a gzip file: " + err.Error())

@@ -43,14 +70,21 @@ func extractTarball(r io.Reader, dir string, owner *[2]int) error {
43	70		return err
44	71		}
45	72		defer func() { _ = root.Close() }()
46		-	tr := tar.NewReader(zr)
47		-	entries, total := 0, int64(0)
	73	+	tr := tar.NewReader(&capReader{r: zr, n: maxStreamBytes})
	74	+	entries, total, top := 0, int64(0), ""
48	75		for {
	76	+	if err := ctx.Err(); err != nil {
	77	+	return err
	78	+	}
49	79		h, err := tr.Next()
50	80		if errors.Is(err, io.EOF) {
51	81		return nil
52	82		}
53	83		if err != nil {
	84	+	var ea errArchive
	85	+	if errors.As(err, &ea) {
	86	+	return ea
	87	+	}
54	88		return errArchive("the archive is not a valid tar file: " + err.Error())
55	89		}
56	90		if entries++; entries > maxArchiveEntries {

@@ -59,12 +93,35 @@ func extractTarball(r io.Reader, dir string, owner *[2]int) error {
59	93		if total += h.Size; total > maxArchiveBytes {
60	94		return errArchive("the archive is too large once unpacked")
61	95		}
62		-	if err := extractEntry(root, tr, h, owner); err != nil {
	96	+	if err := extractEntry(root, tr, h, owner, &top); err != nil {
63	97		return err
64	98		}
65	99		}
66	100		}
67	101
	102	+	// stripTop validates the name of an entry, drops its top directory and returns
	103	+	// the rest, clean and relative to the final root ("" for the top directory
	104	+	// itself). All entries must share one top directory.
	105	+	func stripTop(name string, top *string) (string, error) {
	106	+	c, err := cleanEntry(name)
	107	+	if err != nil {
	108	+	return "", err
	109	+	}
	110	+	if c == "" {
	111	+	return "", nil
	112	+	}
	113	+	first, rest, hasRest := strings.Cut(c, "/")
	114	+	if *top == "" {
	115	+	*top = first
	116	+	} else if first != *top {
	117	+	return "", errArchive(fmt.Sprintf("the archive has more than one top-level directory: %q and %q", *top, first))
	118	+	}
	119	+	if !hasRest {
	120	+	return "", nil
	121	+	}
	122	+	return rest, nil
	123	+	}
	124	+
68	125		// cleanEntry validates the name of an entry and returns it clean and relative.
69	126		func cleanEntry(name string) (string, error) {
70	127		if strings.ContainsRune(name, 0) \|\| strings.HasPrefix(name, "/") {

@@ -97,15 +154,21 @@ func linkInside(name, target string, hard bool) bool {
97	154		return t != ".." && !strings.HasPrefix(t, "../")
98	155		}
99	156
100		-	func extractEntry(root os.Root, tr tar.Reader, h tar.Header, owner [2]int) error {
	157	+	func extractEntry(root os.Root, tr tar.Reader, h tar.Header, owner [2]int, top *string) error {
101	158		switch h.Typeflag {
102	159		case tar.TypeXGlobalHeader, tar.TypeXHeader:
103	160		return nil // git archive's pax_global_header: the commit id
104	161		}
105		-	name, err := cleanEntry(h.Name)
106		-	if err != nil \|\| name == "" {
	162	+	name, err := stripTop(h.Name, top)
	163	+	if err != nil {
107	164		return err
108	165		}
	166	+	if name == "" {
	167	+	if h.Typeflag != tar.TypeDir {
	168	+	return errArchive(fmt.Sprintf("the archive has a file next to its top-level directory: %q", h.Name))
	169	+	}
	170	+	return nil
	171	+	}
109	172		perm := os.FileMode(h.Mode) & 0o777
110	173		chown := func() error {
111	174		if owner == nil {

@@ -154,7 +217,7 @@ func extractEntry(root os.Root, tr tar.Reader, h tar.Header, owner [2]int) e
154	217		}
155	218		return chown()
156	219		case tar.TypeLink:
157		-	tgt, err := cleanEntry(h.Linkname)
	220	+	tgt, err := stripTop(h.Linkname, top)
158	221		if err != nil \|\| tgt == "" \|\| !linkInside(name, tgt, true) {
159	222		return errArchive(fmt.Sprintf("the archive has a link that points outside of it: %q -> %q", name, h.Linkname))
160	223		}

 import (
 	"archive/tar"
 	"compress/gzip"
+	"context"
 	"errors"
 	"fmt"
 	"io"
 )
 // Limits of an action archive. The runner has none (it runs tar); these keep a
-// hostile archive from filling the sandbox.
+// hostile archive from filling the sandbox. They are variables for the tests.
-const (
+var (
 	maxArchiveEntries = 200_000
-	maxArchiveBytes   = 2 << 30 // unpacked
+	// maxArchiveBytes bounds the sizes of the regular files; maxStreamBytes the
+	// whole decompressed tar stream, headers and PAX/GNU long-name records
+	// included, which the entry and size counters do not see.
+	maxArchiveBytes = int64(2 << 30)
+	maxStreamBytes  = maxArchiveBytes + 64<<20
 )
 // errArchive is returned for an archive that is not safe or not valid. Its text
 func (e errArchive) Error() string { return string(e) }
+// capReader fails once more than n bytes were read.
+type capReader struct {
+	r io.Reader
+	n int64
+}
+func (c *capReader) Read(p []byte) (int, error) {
+	if c.n <= 0 {
+		return 0, errArchive("the archive is too large once unpacked")
+	}
+	if int64(len(p)) > c.n {
+		p = p[:c.n]
+	}
+	n, err := c.r.Read(p)
+	c.n -= int64(n)
+	return n, err
+}
 // extractTarball unpacks a gzip-compressed tar into dir, which must exist and
-// be empty. Every name is checked: no absolute path, no "..", no link whose
+// be empty, and strips the single top directory of the archive (GitHub's
-// target leaves dir, and everything is written through an os.Root, so that a
+// archives have exactly one, ActionManager.cs:1380-1393): the content lands in
-// link planted by an earlier entry cannot redirect a later one. owner, when
+// dir itself, so no file moves afterwards and a link that is valid here stays
-// set, is the uid and gid of the files.
+// valid. Every name is checked: no absolute path, no "..", one top directory,
-func extractTarball(r io.Reader, dir string, owner *[2]int) error {
+// no link whose target leaves dir, and everything is written through an
+// os.Root, so that a link planted by an earlier entry cannot redirect a later
+// one. The decompressed stream is capped, and ctx is checked between entries.
+// owner, when set, is the uid and gid of the files.
+func extractTarball(ctx context.Context, r io.Reader, dir string, owner *[2]int) error {
 	zr, err := gzip.NewReader(r)
 	if err != nil {
 		return errArchive("the archive is not a gzip file: " + err.Error())
 		return err
 	}
 	defer func() { _ = root.Close() }()
-	tr := tar.NewReader(zr)
+	tr := tar.NewReader(&capReader{r: zr, n: maxStreamBytes})
-	entries, total := 0, int64(0)
+	entries, total, top := 0, int64(0), ""
 	for {
+		if err := ctx.Err(); err != nil {
+			return err
+		}
 		h, err := tr.Next()
 		if errors.Is(err, io.EOF) {
 			return nil
 		}
 		if err != nil {
+			var ea errArchive
+			if errors.As(err, &ea) {
+				return ea
+			}
 			return errArchive("the archive is not a valid tar file: " + err.Error())
 		}
 		if entries++; entries > maxArchiveEntries {
 		if total += h.Size; total > maxArchiveBytes {
 			return errArchive("the archive is too large once unpacked")
 		}
-		if err := extractEntry(root, tr, h, owner); err != nil {
+		if err := extractEntry(root, tr, h, owner, &top); err != nil {
 			return err
 		}
 	}
 }
+// stripTop validates the name of an entry, drops its top directory and returns
+// the rest, clean and relative to the final root ("" for the top directory
+// itself). All entries must share one top directory.
+func stripTop(name string, top *string) (string, error) {
+	c, err := cleanEntry(name)
+	if err != nil {
+		return "", err
+	}
+	if c == "" {
+		return "", nil
+	}
+	first, rest, hasRest := strings.Cut(c, "/")
+	if *top == "" {
+		*top = first
+	} else if first != *top {
+		return "", errArchive(fmt.Sprintf("the archive has more than one top-level directory: %q and %q", *top, first))
+	}
+	if !hasRest {
+		return "", nil
+	}
+	return rest, nil
+}
 // cleanEntry validates the name of an entry and returns it clean and relative.
 func cleanEntry(name string) (string, error) {
 	if strings.ContainsRune(name, 0) || strings.HasPrefix(name, "/") {
 	return t != ".." && !strings.HasPrefix(t, "../")
 }
-func extractEntry(root *os.Root, tr *tar.Reader, h *tar.Header, owner *[2]int) error {
+func extractEntry(root *os.Root, tr *tar.Reader, h *tar.Header, owner *[2]int, top *string) error {
 	switch h.Typeflag {
 	case tar.TypeXGlobalHeader, tar.TypeXHeader:
 		return nil // git archive's pax_global_header: the commit id
 	}
-	name, err := cleanEntry(h.Name)
+	name, err := stripTop(h.Name, top)
-	if err != nil || name == "" {
+	if err != nil {
 		return err
 	}
+	if name == "" {
+		if h.Typeflag != tar.TypeDir {
+			return errArchive(fmt.Sprintf("the archive has a file next to its top-level directory: %q", h.Name))
+		}
+		return nil
+	}
 	perm := os.FileMode(h.Mode) & 0o777
 	chown := func() error {
 		if owner == nil {
 		}
 		return chown()
 	case tar.TypeLink:
-		tgt, err := cleanEntry(h.Linkname)
+		tgt, err := stripTop(h.Linkname, top)
 		if err != nil || tgt == "" || !linkInside(name, tgt, true) {
 			return errArchive(fmt.Sprintf("the archive has a link that points outside of it: %q -> %q", name, h.Linkname))
 		}

M internal/executor/actions_extract_test.go +108 -18


@@ -6,6 +6,7 @@ import (
6	6		"archive/tar"
7	7		"bytes"
8	8		"compress/gzip"
	9	+	"context"
9	10		"os"
10	11		"path/filepath"
11	12		"strings"

@@ -48,33 +49,43 @@ func tarball(t testing.T, ents ...tent) bytes.Reader {
48	49
49	50		func TestExtractTarball(t *testing.T) {
50	51		dir := t.TempDir()
51		-	err := extractTarball(tarball(t,
	52	+	err := extractTarball(context.Background(), tarball(t,
52	53		tent{name: "o-r-abc/", typ: tar.TypeDir},
53	54		tent{name: "o-r-abc/action.yml", body: "name: x"},
54	55		tent{name: "o-r-abc/sub/main.js", body: "1"},
55	56		tent{name: "o-r-abc/link", typ: tar.TypeSymlink, link: "sub/main.js"},
	57	+	tent{name: "o-r-abc/hard", typ: tar.TypeLink, link: "o-r-abc/sub/main.js"},
56	58		), dir, nil)
57	59		if err != nil {
58	60		t.Fatal(err)
59	61		}
60		-	if b, _ := os.ReadFile(filepath.Join(dir, "o-r-abc", "action.yml")); string(b) != "name: x" {
61		-	t.Errorf("action.yml: %q", b)
	62	+	// The top directory is stripped: the content is in dir itself.
	63	+	for _, f := range []string{"action.yml", "link", "hard"} {
	64	+	if _, err := os.Stat(filepath.Join(dir, f)); err != nil {
	65	+	t.Errorf("%s: %v", f, err)
	66	+	}
62	67		}
63		-	if b, _ := os.ReadFile(filepath.Join(dir, "o-r-abc", "link")); string(b) != "1" {
	68	+	if b, _ := os.ReadFile(filepath.Join(dir, "link")); string(b) != "1" {
64	69		t.Errorf("link: %q", b)
65	70		}
	71	+	if _, err := os.Stat(filepath.Join(dir, "o-r-abc")); err == nil {
	72	+	t.Error("the top directory must be stripped")
	73	+	}
66	74		}
67	75
68	76		func TestExtractTarballRejects(t *testing.T) {
69	77		cases := map[string][]tent{
70		-	"absolute": {{name: "/etc/passwd", body: "x"}},
71		-	"dotdot": {{name: "a/../../x", body: "x"}},
72		-	"symlink out": {{name: "a/l", typ: tar.TypeSymlink, link: "../../x"}},
73		-	"symlink abs": {{name: "l", typ: tar.TypeSymlink, link: "/etc"}},
74		-	"hardlink out": {{name: "l", typ: tar.TypeLink, link: "../x"}},
75		-	"hardlink abs": {{name: "l", typ: tar.TypeLink, link: "/etc/passwd"}},
76		-	"through link": {{name: "d", typ: tar.TypeSymlink, link: "."}, {name: "d/../../x", body: "x"}},
77		-	"link then dir": {{name: "l", typ: tar.TypeSymlink, link: "a/.."}, {name: "l/../x", body: "x"}},
	78	+	"absolute": {{name: "/etc/passwd", body: "x"}},
	79	+	"dotdot": {{name: "t/a/../../x", body: "x"}},
	80	+	"symlink out": {{name: "t/a/l", typ: tar.TypeSymlink, link: "../../x"}},
	81	+	"symlink abs": {{name: "t/l", typ: tar.TypeSymlink, link: "/etc"}},
	82	+	"hardlink out": {{name: "t/l", typ: tar.TypeLink, link: "../x"}},
	83	+	"hardlink abs": {{name: "t/l", typ: tar.TypeLink, link: "/etc/passwd"}},
	84	+	"through link": {{name: "t/d", typ: tar.TypeSymlink, link: "."}, {name: "t/d/../../x", body: "x"}},
	85	+	"link then dir": {{name: "t/l", typ: tar.TypeSymlink, link: "a/.."}, {name: "t/l/../x", body: "x"}},
	86	+	"two tops": {{name: "t/a", body: "1"}, {name: "u/b", body: "2"}},
	87	+	"file beside": {{name: "t/", typ: tar.TypeDir}, {name: "loose", body: "x"}},
	88	+	"hardlink other": {{name: "t/a", body: "1"}, {name: "t/l", typ: tar.TypeLink, link: "u/a"}},
78	89		}
79	90		for name, ents := range cases {
80	91		t.Run(name, func(t *testing.T) {

@@ -83,7 +94,7 @@ func TestExtractTarballRejects(t *testing.T) {
83	94		if err := os.Mkdir(dir, 0o755); err != nil {
84	95		t.Fatal(err)
85	96		}
86		-	err := extractTarball(tarball(t, ents...), dir, nil)
	97	+	err := extractTarball(context.Background(), tarball(t, ents...), dir, nil)
87	98		if err == nil {
88	99		t.Fatal("accepted")
89	100		}

@@ -94,8 +105,51 @@ func TestExtractTarballRejects(t *testing.T) {
94	105		}
95	106		}
96	107
	108	+	// GO-001: "top/link -> ../outside" stays inside the staging directory of the
	109	+	// extraction but escapes once the top directory is stripped. The target is
	110	+	// resolved against the final root, whatever the location the files are
	111	+	// extracted to, and the relocated tree has no link that leaves it.
	112	+	func TestExtractTarballLinkEscapesStrippedRoot(t *testing.T) {
	113	+	// From top/sub/l, i.e. sub/l once the top directory is stripped.
	114	+	for link, ok := range map[string]bool{"../outside": true, "..": true, "../x": true, "../../outside": false, "../../top/x": false, "../../../x": false} {
	115	+	dir := filepath.Join(t.TempDir(), "stage")
	116	+	_ = os.Mkdir(dir, 0o755)
	117	+	err := extractTarball(context.Background(), tarball(t,
	118	+	tent{name: "top/x", body: "x"},
	119	+	tent{name: "top/sub/l", typ: tar.TypeSymlink, link: link},
	120	+	), dir, nil)
	121	+	if (err == nil) != ok {
	122	+	t.Errorf("link %q: accepted=%v, want %v (%v)", link, err == nil, ok, err)
	123	+	}
	124	+	}
	125	+	// The direct case of the finding.
	126	+	dir := filepath.Join(t.TempDir(), "stage")
	127	+	_ = os.Mkdir(dir, 0o755)
	128	+	if err := extractTarball(context.Background(), tarball(t,
	129	+	tent{name: "top/link", typ: tar.TypeSymlink, link: "../outside"},
	130	+	), dir, nil); err == nil {
	131	+	t.Fatal("top/link -> ../outside was accepted")
	132	+	}
	133	+	// A tree that was accepted keeps all its links inside after relocation.
	134	+	stage := filepath.Join(t.TempDir(), "stage")
	135	+	_ = os.Mkdir(stage, 0o755)
	136	+	if err := extractTarball(context.Background(), tarball(t,
	137	+	tent{name: "top/real", body: "r"},
	138	+	tent{name: "top/a/b/l", typ: tar.TypeSymlink, link: "../../real"},
	139	+	), stage, nil); err != nil {
	140	+	t.Fatal(err)
	141	+	}
	142	+	final := filepath.Join(t.TempDir(), "final")
	143	+	if err := os.Rename(stage, final); err != nil {
	144	+	t.Fatal(err)
	145	+	}
	146	+	if b, err := os.ReadFile(filepath.Join(final, "a", "b", "l")); err != nil \|\| string(b) != "r" {
	147	+	t.Fatalf("relocated link: %q %v", b, err)
	148	+	}
	149	+	}
	150	+
97	151		func TestExtractTarballNotGzip(t *testing.T) {
98		-	err := extractTarball(strings.NewReader("not gzip"), t.TempDir(), nil)
	152	+	err := extractTarball(context.Background(), strings.NewReader("not gzip"), t.TempDir(), nil)
99	153		if err == nil \|\| !strings.Contains(err.Error(), "not a gzip") {
100	154		t.Fatalf("%v", err)
101	155		}

@@ -107,10 +161,10 @@ func TestExtractTarballLinkChain(t *testing.T) {
107	161		parent := t.TempDir()
108	162		dir := filepath.Join(parent, "dest")
109	163		_ = os.Mkdir(dir, 0o755)
110		-	err := extractTarball(tarball(t,
111		-	tent{name: "a", typ: tar.TypeSymlink, link: "b"},
112		-	tent{name: "b", typ: tar.TypeSymlink, link: "."},
113		-	tent{name: "a/a/a/f", body: "x"},
	164	+	err := extractTarball(context.Background(), tarball(t,
	165	+	tent{name: "t/a", typ: tar.TypeSymlink, link: "b"},
	166	+	tent{name: "t/b", typ: tar.TypeSymlink, link: "."},
	167	+	tent{name: "t/a/a/a/f", body: "x"},
114	168		), dir, nil)
115	169		if err != nil {
116	170		t.Fatal(err)

@@ -119,3 +173,39 @@ func TestExtractTarballLinkChain(t *testing.T) {
119	173		t.Fatalf("f: %q", b)
120	174		}
121	175		}
	176	+
	177	+	// GO-004: PAX and GNU long-name records are not entries and have no Size in the
	178	+	// header Next returns, so the counters do not see them. The decompressed stream
	179	+	// is capped.
	180	+	func TestExtractTarballMetadataFlood(t *testing.T) {
	181	+	old := maxStreamBytes
	182	+	maxStreamBytes = 256 << 10
	183	+	defer func() { maxStreamBytes = old }()
	184	+	var buf bytes.Buffer
	185	+	zw := gzip.NewWriter(&buf)
	186	+	tw := tar.NewWriter(zw)
	187	+	for i := 0; i < 400; i++ {
	188	+	// A PAX record per file with a 4 KiB value; the entries stay below
	189	+	// every counter.
	190	+	h := &tar.Header{Name: "t/f", Typeflag: tar.TypeReg, Mode: 0o644, Size: 0, Format: tar.FormatPAX,
	191	+	PAXRecords: map[string]string{"comment": strings.Repeat("x", 4096)}}
	192	+	if err := tw.WriteHeader(h); err != nil {
	193	+	t.Fatal(err)
	194	+	}
	195	+	}
	196	+	_ = tw.Close()
	197	+	_ = zw.Close()
	198	+	err := extractTarball(context.Background(), bytes.NewReader(buf.Bytes()), t.TempDir(), nil)
	199	+	if err == nil \|\| !strings.Contains(err.Error(), "too large once unpacked") {
	200	+	t.Fatalf("%v", err)
	201	+	}
	202	+	}
	203	+
	204	+	func TestExtractTarballCancelled(t *testing.T) {
	205	+	ctx, cancel := context.WithCancel(context.Background())
	206	+	cancel()
	207	+	err := extractTarball(ctx, tarball(t, tent{name: "t/a", body: "1"}), t.TempDir(), nil)
	208	+	if err != context.Canceled {
	209	+	t.Fatalf("%v", err)
	210	+	}
	211	+	}


@@ -6,6 +6,7 @@ import (
6		"archive/tar"	6		"archive/tar"
7		"bytes"	7		"bytes"
8		"compress/gzip"	8		"compress/gzip"
			9	+	"context"
9		"os"	10		"os"
10		"path/filepath"	11		"path/filepath"
11		"strings"	12		"strings"

@@ -48,33 +49,43 @@ func tarball(t testing.T, ents ...tent) bytes.Reader {
48			49
49		func TestExtractTarball(t *testing.T) {	50		func TestExtractTarball(t *testing.T) {
50		dir := t.TempDir()	51		dir := t.TempDir()
51	-	err := extractTarball(tarball(t,	52	+	err := extractTarball(context.Background(), tarball(t,
52		tent{name: "o-r-abc/", typ: tar.TypeDir},	53		tent{name: "o-r-abc/", typ: tar.TypeDir},
53		tent{name: "o-r-abc/action.yml", body: "name: x"},	54		tent{name: "o-r-abc/action.yml", body: "name: x"},
54		tent{name: "o-r-abc/sub/main.js", body: "1"},	55		tent{name: "o-r-abc/sub/main.js", body: "1"},
55		tent{name: "o-r-abc/link", typ: tar.TypeSymlink, link: "sub/main.js"},	56		tent{name: "o-r-abc/link", typ: tar.TypeSymlink, link: "sub/main.js"},
			57	+	tent{name: "o-r-abc/hard", typ: tar.TypeLink, link: "o-r-abc/sub/main.js"},
56		), dir, nil)	58		), dir, nil)
57		if err != nil {	59		if err != nil {
58		t.Fatal(err)	60		t.Fatal(err)
59		}	61		}
60	-	if b, _ := os.ReadFile(filepath.Join(dir, "o-r-abc", "action.yml")); string(b) != "name: x" {	62	+	// The top directory is stripped: the content is in dir itself.
61	-	t.Errorf("action.yml: %q", b)	63	+	for _, f := range []string{"action.yml", "link", "hard"} {
			64	+	if _, err := os.Stat(filepath.Join(dir, f)); err != nil {
			65	+	t.Errorf("%s: %v", f, err)
			66	+	}
62		}	67		}
63	-	if b, _ := os.ReadFile(filepath.Join(dir, "o-r-abc", "link")); string(b) != "1" {	68	+	if b, _ := os.ReadFile(filepath.Join(dir, "link")); string(b) != "1" {
64		t.Errorf("link: %q", b)	69		t.Errorf("link: %q", b)
65		}	70		}
			71	+	if _, err := os.Stat(filepath.Join(dir, "o-r-abc")); err == nil {
			72	+	t.Error("the top directory must be stripped")
			73	+	}
66		}	74		}
67			75
68		func TestExtractTarballRejects(t *testing.T) {	76		func TestExtractTarballRejects(t *testing.T) {
69		cases := map[string][]tent{	77		cases := map[string][]tent{
70	-	"absolute": {{name: "/etc/passwd", body: "x"}},	78	+	"absolute": {{name: "/etc/passwd", body: "x"}},
71	-	"dotdot": {{name: "a/../../x", body: "x"}},	79	+	"dotdot": {{name: "t/a/../../x", body: "x"}},
72	-	"symlink out": {{name: "a/l", typ: tar.TypeSymlink, link: "../../x"}},	80	+	"symlink out": {{name: "t/a/l", typ: tar.TypeSymlink, link: "../../x"}},
73	-	"symlink abs": {{name: "l", typ: tar.TypeSymlink, link: "/etc"}},	81	+	"symlink abs": {{name: "t/l", typ: tar.TypeSymlink, link: "/etc"}},
74	-	"hardlink out": {{name: "l", typ: tar.TypeLink, link: "../x"}},	82	+	"hardlink out": {{name: "t/l", typ: tar.TypeLink, link: "../x"}},
75	-	"hardlink abs": {{name: "l", typ: tar.TypeLink, link: "/etc/passwd"}},	83	+	"hardlink abs": {{name: "t/l", typ: tar.TypeLink, link: "/etc/passwd"}},
76	-	"through link": {{name: "d", typ: tar.TypeSymlink, link: "."}, {name: "d/../../x", body: "x"}},	84	+	"through link": {{name: "t/d", typ: tar.TypeSymlink, link: "."}, {name: "t/d/../../x", body: "x"}},
77	-	"link then dir": {{name: "l", typ: tar.TypeSymlink, link: "a/.."}, {name: "l/../x", body: "x"}},	85	+	"link then dir": {{name: "t/l", typ: tar.TypeSymlink, link: "a/.."}, {name: "t/l/../x", body: "x"}},
			86	+	"two tops": {{name: "t/a", body: "1"}, {name: "u/b", body: "2"}},
			87	+	"file beside": {{name: "t/", typ: tar.TypeDir}, {name: "loose", body: "x"}},
			88	+	"hardlink other": {{name: "t/a", body: "1"}, {name: "t/l", typ: tar.TypeLink, link: "u/a"}},
78		}	89		}
79		for name, ents := range cases {	90		for name, ents := range cases {
80		t.Run(name, func(t *testing.T) {	91		t.Run(name, func(t *testing.T) {

@@ -83,7 +94,7 @@ func TestExtractTarballRejects(t *testing.T) {
83		if err := os.Mkdir(dir, 0o755); err != nil {	94		if err := os.Mkdir(dir, 0o755); err != nil {
84		t.Fatal(err)	95		t.Fatal(err)
85		}	96		}
86	-	err := extractTarball(tarball(t, ents...), dir, nil)	97	+	err := extractTarball(context.Background(), tarball(t, ents...), dir, nil)
87		if err == nil {	98		if err == nil {
88		t.Fatal("accepted")	99		t.Fatal("accepted")
89		}	100		}

@@ -94,8 +105,51 @@ func TestExtractTarballRejects(t *testing.T) {
94		}	105		}
95		}	106		}
96			107
			108	+	// GO-001: "top/link -> ../outside" stays inside the staging directory of the
			109	+	// extraction but escapes once the top directory is stripped. The target is
			110	+	// resolved against the final root, whatever the location the files are
			111	+	// extracted to, and the relocated tree has no link that leaves it.
			112	+	func TestExtractTarballLinkEscapesStrippedRoot(t *testing.T) {
			113	+	// From top/sub/l, i.e. sub/l once the top directory is stripped.
			114	+	for link, ok := range map[string]bool{"../outside": true, "..": true, "../x": true, "../../outside": false, "../../top/x": false, "../../../x": false} {
			115	+	dir := filepath.Join(t.TempDir(), "stage")
			116	+	_ = os.Mkdir(dir, 0o755)
			117	+	err := extractTarball(context.Background(), tarball(t,
			118	+	tent{name: "top/x", body: "x"},
			119	+	tent{name: "top/sub/l", typ: tar.TypeSymlink, link: link},
			120	+	), dir, nil)
			121	+	if (err == nil) != ok {
			122	+	t.Errorf("link %q: accepted=%v, want %v (%v)", link, err == nil, ok, err)
			123	+	}
			124	+	}
			125	+	// The direct case of the finding.
			126	+	dir := filepath.Join(t.TempDir(), "stage")
			127	+	_ = os.Mkdir(dir, 0o755)
			128	+	if err := extractTarball(context.Background(), tarball(t,
			129	+	tent{name: "top/link", typ: tar.TypeSymlink, link: "../outside"},
			130	+	), dir, nil); err == nil {
			131	+	t.Fatal("top/link -> ../outside was accepted")
			132	+	}
			133	+	// A tree that was accepted keeps all its links inside after relocation.
			134	+	stage := filepath.Join(t.TempDir(), "stage")
			135	+	_ = os.Mkdir(stage, 0o755)
			136	+	if err := extractTarball(context.Background(), tarball(t,
			137	+	tent{name: "top/real", body: "r"},
			138	+	tent{name: "top/a/b/l", typ: tar.TypeSymlink, link: "../../real"},
			139	+	), stage, nil); err != nil {
			140	+	t.Fatal(err)
			141	+	}
			142	+	final := filepath.Join(t.TempDir(), "final")
			143	+	if err := os.Rename(stage, final); err != nil {
			144	+	t.Fatal(err)
			145	+	}
			146	+	if b, err := os.ReadFile(filepath.Join(final, "a", "b", "l")); err != nil \|\| string(b) != "r" {
			147	+	t.Fatalf("relocated link: %q %v", b, err)
			148	+	}
			149	+	}
			150	+
97		func TestExtractTarballNotGzip(t *testing.T) {	151		func TestExtractTarballNotGzip(t *testing.T) {
98	-	err := extractTarball(strings.NewReader("not gzip"), t.TempDir(), nil)	152	+	err := extractTarball(context.Background(), strings.NewReader("not gzip"), t.TempDir(), nil)
99		if err == nil \|\| !strings.Contains(err.Error(), "not a gzip") {	153		if err == nil \|\| !strings.Contains(err.Error(), "not a gzip") {
100		t.Fatalf("%v", err)	154		t.Fatalf("%v", err)
101		}	155		}

@@ -107,10 +161,10 @@ func TestExtractTarballLinkChain(t *testing.T) {
107		parent := t.TempDir()	161		parent := t.TempDir()
108		dir := filepath.Join(parent, "dest")	162		dir := filepath.Join(parent, "dest")
109		_ = os.Mkdir(dir, 0o755)	163		_ = os.Mkdir(dir, 0o755)
110	-	err := extractTarball(tarball(t,	164	+	err := extractTarball(context.Background(), tarball(t,
111	-	tent{name: "a", typ: tar.TypeSymlink, link: "b"},	165	+	tent{name: "t/a", typ: tar.TypeSymlink, link: "b"},
112	-	tent{name: "b", typ: tar.TypeSymlink, link: "."},	166	+	tent{name: "t/b", typ: tar.TypeSymlink, link: "."},
113	-	tent{name: "a/a/a/f", body: "x"},	167	+	tent{name: "t/a/a/a/f", body: "x"},
114		), dir, nil)	168		), dir, nil)
115		if err != nil {	169		if err != nil {
116		t.Fatal(err)	170		t.Fatal(err)

@@ -119,3 +173,39 @@ func TestExtractTarballLinkChain(t *testing.T) {
119		t.Fatalf("f: %q", b)	173		t.Fatalf("f: %q", b)
120		}	174		}
121		}	175		}
			176	+
			177	+	// GO-004: PAX and GNU long-name records are not entries and have no Size in the
			178	+	// header Next returns, so the counters do not see them. The decompressed stream
			179	+	// is capped.
			180	+	func TestExtractTarballMetadataFlood(t *testing.T) {
			181	+	old := maxStreamBytes
			182	+	maxStreamBytes = 256 << 10
			183	+	defer func() { maxStreamBytes = old }()
			184	+	var buf bytes.Buffer
			185	+	zw := gzip.NewWriter(&buf)
			186	+	tw := tar.NewWriter(zw)
			187	+	for i := 0; i < 400; i++ {
			188	+	// A PAX record per file with a 4 KiB value; the entries stay below
			189	+	// every counter.
			190	+	h := &tar.Header{Name: "t/f", Typeflag: tar.TypeReg, Mode: 0o644, Size: 0, Format: tar.FormatPAX,
			191	+	PAXRecords: map[string]string{"comment": strings.Repeat("x", 4096)}}
			192	+	if err := tw.WriteHeader(h); err != nil {
			193	+	t.Fatal(err)
			194	+	}
			195	+	}
			196	+	_ = tw.Close()
			197	+	_ = zw.Close()
			198	+	err := extractTarball(context.Background(), bytes.NewReader(buf.Bytes()), t.TempDir(), nil)
			199	+	if err == nil \|\| !strings.Contains(err.Error(), "too large once unpacked") {
			200	+	t.Fatalf("%v", err)
			201	+	}
			202	+	}
			203	+
			204	+	func TestExtractTarballCancelled(t *testing.T) {
			205	+	ctx, cancel := context.WithCancel(context.Background())
			206	+	cancel()
			207	+	err := extractTarball(ctx, tarball(t, tent{name: "t/a", body: "1"}), t.TempDir(), nil)
			208	+	if err != context.Canceled {
			209	+	t.Fatalf("%v", err)
			210	+	}
			211	+	}