detached 1396410

feat(executor): extract action archives with traversal and link checks

commit
1396410004b6be0a615e3f993962bb4e94de7cf6
author
brice · · 2026-10-13 15:16 UTC
parent
c4ec791
signature
GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email
2 files · +286 -0
Unified
UnifiedSplit
A internal/executor/actions_extract.go +165 -0
@@ -0,0 +1,165 @@
1+// SPDX-License-Identifier: MIT
2+
3+package executor
4+
5+import (
6+ "archive/tar"
7+ "compress/gzip"
8+ "errors"
9+ "fmt"
10+ "io"
11+ "os"
12+ "path"
13+ "strings"
14+)
15+
16+// Limits of an action archive. The runner has none (it runs tar); these keep a
17+// hostile archive from filling the sandbox.
18+const (
19+ maxArchiveEntries = 200_000
20+ maxArchiveBytes = 2 << 30 // unpacked
21+)
22+
23+// errArchive is returned for an archive that is not safe or not valid. Its text
24+// is ermite's: GitHub's runner runs tar, which accepts absolute paths and links
25+// out of the tree (Runner.Worker/ActionManager.cs:1358-1394).
26+type errArchive string
27+
28+func (e errArchive) Error() string { return string(e) }
29+
30+// 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 {
36+ zr, err := gzip.NewReader(r)
37+ if err != nil {
38+ return errArchive("the archive is not a gzip file: " + err.Error())
39+ }
40+ defer func() { _ = zr.Close() }()
41+ root, err := os.OpenRoot(dir)
42+ if err != nil {
43+ return err
44+ }
45+ defer func() { _ = root.Close() }()
46+ tr := tar.NewReader(zr)
47+ entries, total := 0, int64(0)
48+ for {
49+ h, err := tr.Next()
50+ if errors.Is(err, io.EOF) {
51+ return nil
52+ }
53+ if err != nil {
54+ return errArchive("the archive is not a valid tar file: " + err.Error())
55+ }
56+ if entries++; entries > maxArchiveEntries {
57+ return errArchive("the archive has too many entries")
58+ }
59+ if total += h.Size; total > maxArchiveBytes {
60+ return errArchive("the archive is too large once unpacked")
61+ }
62+ if err := extractEntry(root, tr, h, owner); err != nil {
63+ return err
64+ }
65+ }
66+}
67+
68+// cleanEntry validates the name of an entry and returns it clean and relative.
69+func cleanEntry(name string) (string, error) {
70+ if strings.ContainsRune(name, 0) || strings.HasPrefix(name, "/") {
71+ return "", errArchive(fmt.Sprintf("the archive has an absolute or invalid path: %q", name))
72+ }
73+ for _, seg := range strings.Split(name, "/") {
74+ if seg == ".." {
75+ return "", errArchive(fmt.Sprintf("the archive has a path that leaves its directory: %q", name))
76+ }
77+ }
78+ c := path.Clean(name)
79+ if c == "." {
80+ return "", nil
81+ }
82+ return c, nil
83+}
84+
85+// linkInside reports whether a link at name pointing to target stays in the
86+// tree: target is relative to the directory of name for a symlink, and to the
87+// root for a hard link.
88+func linkInside(name, target string, hard bool) bool {
89+ if target == "" || strings.HasPrefix(target, "/") || strings.ContainsRune(target, 0) {
90+ return false
91+ }
92+ base := path.Dir(name)
93+ if hard {
94+ base = "."
95+ }
96+ t := path.Join(base, target)
97+ return t != ".." && !strings.HasPrefix(t, "../")
98+}
99+
100+func extractEntry(root *os.Root, tr *tar.Reader, h *tar.Header, owner *[2]int) error {
101+ switch h.Typeflag {
102+ case tar.TypeXGlobalHeader, tar.TypeXHeader:
103+ return nil // git archive's pax_global_header: the commit id
104+ }
105+ name, err := cleanEntry(h.Name)
106+ if err != nil || name == "" {
107+ return err
108+ }
109+ perm := os.FileMode(h.Mode) & 0o777
110+ chown := func() error {
111+ if owner == nil {
112+ return nil
113+ }
114+ return root.Lchown(name, owner[0], owner[1])
115+ }
116+ if dir := path.Dir(name); dir != "." {
117+ if err := root.MkdirAll(dir, 0o755); err != nil {
118+ return errArchive(fmt.Sprintf("cannot create the directory of %q: %v", name, err))
119+ }
120+ }
121+ switch h.Typeflag {
122+ case tar.TypeDir:
123+ if err := root.MkdirAll(name, 0o755); err != nil {
124+ return err
125+ }
126+ if err := root.Chmod(name, perm|0o700); err != nil {
127+ return err
128+ }
129+ return chown()
130+ case tar.TypeReg:
131+ _ = root.Remove(name)
132+ f, err := root.OpenFile(name, os.O_WRONLY|os.O_CREATE|os.O_EXCL, perm|0o600)
133+ if err != nil {
134+ return errArchive(fmt.Sprintf("cannot create %q: %v", name, err))
135+ }
136+ if _, err := io.Copy(f, tr); err != nil {
137+ _ = f.Close()
138+ return errArchive("the archive is not a valid tar file: " + err.Error())
139+ }
140+ if err := f.Close(); err != nil {
141+ return err
142+ }
143+ if err := root.Chmod(name, perm|0o600); err != nil {
144+ return err
145+ }
146+ return chown()
147+ case tar.TypeSymlink:
148+ if !linkInside(name, h.Linkname, false) {
149+ return errArchive(fmt.Sprintf("the archive has a link that points outside of it: %q -> %q", name, h.Linkname))
150+ }
151+ _ = root.Remove(name)
152+ if err := root.Symlink(h.Linkname, name); err != nil {
153+ return err
154+ }
155+ return chown()
156+ case tar.TypeLink:
157+ tgt, err := cleanEntry(h.Linkname)
158+ if err != nil || tgt == "" || !linkInside(name, tgt, true) {
159+ return errArchive(fmt.Sprintf("the archive has a link that points outside of it: %q -> %q", name, h.Linkname))
160+ }
161+ _ = root.Remove(name)
162+ return root.Link(tgt, name)
163+ }
164+ return nil // devices, fifos: skipped
165+}
@@ -0,0 +1,165 @@
1+// SPDX-License-Identifier: MIT
2+
3+package executor
4+
5+import (
6+ "archive/tar"
7+ "compress/gzip"
8+ "errors"
9+ "fmt"
10+ "io"
11+ "os"
12+ "path"
13+ "strings"
14+)
15+
16+// Limits of an action archive. The runner has none (it runs tar); these keep a
17+// hostile archive from filling the sandbox.
18+const (
19+ maxArchiveEntries = 200_000
20+ maxArchiveBytes = 2 << 30 // unpacked
21+)
22+
23+// errArchive is returned for an archive that is not safe or not valid. Its text
24+// is ermite's: GitHub's runner runs tar, which accepts absolute paths and links
25+// out of the tree (Runner.Worker/ActionManager.cs:1358-1394).
26+type errArchive string
27+
28+func (e errArchive) Error() string { return string(e) }
29+
30+// 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 {
36+ zr, err := gzip.NewReader(r)
37+ if err != nil {
38+ return errArchive("the archive is not a gzip file: " + err.Error())
39+ }
40+ defer func() { _ = zr.Close() }()
41+ root, err := os.OpenRoot(dir)
42+ if err != nil {
43+ return err
44+ }
45+ defer func() { _ = root.Close() }()
46+ tr := tar.NewReader(zr)
47+ entries, total := 0, int64(0)
48+ for {
49+ h, err := tr.Next()
50+ if errors.Is(err, io.EOF) {
51+ return nil
52+ }
53+ if err != nil {
54+ return errArchive("the archive is not a valid tar file: " + err.Error())
55+ }
56+ if entries++; entries > maxArchiveEntries {
57+ return errArchive("the archive has too many entries")
58+ }
59+ if total += h.Size; total > maxArchiveBytes {
60+ return errArchive("the archive is too large once unpacked")
61+ }
62+ if err := extractEntry(root, tr, h, owner); err != nil {
63+ return err
64+ }
65+ }
66+}
67+
68+// cleanEntry validates the name of an entry and returns it clean and relative.
69+func cleanEntry(name string) (string, error) {
70+ if strings.ContainsRune(name, 0) || strings.HasPrefix(name, "/") {
71+ return "", errArchive(fmt.Sprintf("the archive has an absolute or invalid path: %q", name))
72+ }
73+ for _, seg := range strings.Split(name, "/") {
74+ if seg == ".." {
75+ return "", errArchive(fmt.Sprintf("the archive has a path that leaves its directory: %q", name))
76+ }
77+ }
78+ c := path.Clean(name)
79+ if c == "." {
80+ return "", nil
81+ }
82+ return c, nil
83+}
84+
85+// linkInside reports whether a link at name pointing to target stays in the
86+// tree: target is relative to the directory of name for a symlink, and to the
87+// root for a hard link.
88+func linkInside(name, target string, hard bool) bool {
89+ if target == "" || strings.HasPrefix(target, "/") || strings.ContainsRune(target, 0) {
90+ return false
91+ }
92+ base := path.Dir(name)
93+ if hard {
94+ base = "."
95+ }
96+ t := path.Join(base, target)
97+ return t != ".." && !strings.HasPrefix(t, "../")
98+}
99+
100+func extractEntry(root *os.Root, tr *tar.Reader, h *tar.Header, owner *[2]int) error {
101+ switch h.Typeflag {
102+ case tar.TypeXGlobalHeader, tar.TypeXHeader:
103+ return nil // git archive's pax_global_header: the commit id
104+ }
105+ name, err := cleanEntry(h.Name)
106+ if err != nil || name == "" {
107+ return err
108+ }
109+ perm := os.FileMode(h.Mode) & 0o777
110+ chown := func() error {
111+ if owner == nil {
112+ return nil
113+ }
114+ return root.Lchown(name, owner[0], owner[1])
115+ }
116+ if dir := path.Dir(name); dir != "." {
117+ if err := root.MkdirAll(dir, 0o755); err != nil {
118+ return errArchive(fmt.Sprintf("cannot create the directory of %q: %v", name, err))
119+ }
120+ }
121+ switch h.Typeflag {
122+ case tar.TypeDir:
123+ if err := root.MkdirAll(name, 0o755); err != nil {
124+ return err
125+ }
126+ if err := root.Chmod(name, perm|0o700); err != nil {
127+ return err
128+ }
129+ return chown()
130+ case tar.TypeReg:
131+ _ = root.Remove(name)
132+ f, err := root.OpenFile(name, os.O_WRONLY|os.O_CREATE|os.O_EXCL, perm|0o600)
133+ if err != nil {
134+ return errArchive(fmt.Sprintf("cannot create %q: %v", name, err))
135+ }
136+ if _, err := io.Copy(f, tr); err != nil {
137+ _ = f.Close()
138+ return errArchive("the archive is not a valid tar file: " + err.Error())
139+ }
140+ if err := f.Close(); err != nil {
141+ return err
142+ }
143+ if err := root.Chmod(name, perm|0o600); err != nil {
144+ return err
145+ }
146+ return chown()
147+ case tar.TypeSymlink:
148+ if !linkInside(name, h.Linkname, false) {
149+ return errArchive(fmt.Sprintf("the archive has a link that points outside of it: %q -> %q", name, h.Linkname))
150+ }
151+ _ = root.Remove(name)
152+ if err := root.Symlink(h.Linkname, name); err != nil {
153+ return err
154+ }
155+ return chown()
156+ case tar.TypeLink:
157+ tgt, err := cleanEntry(h.Linkname)
158+ if err != nil || tgt == "" || !linkInside(name, tgt, true) {
159+ return errArchive(fmt.Sprintf("the archive has a link that points outside of it: %q -> %q", name, h.Linkname))
160+ }
161+ _ = root.Remove(name)
162+ return root.Link(tgt, name)
163+ }
164+ return nil // devices, fifos: skipped
165+}
A internal/executor/actions_extract_test.go +121 -0
@@ -0,0 +1,121 @@
1+// SPDX-License-Identifier: MIT
2+
3+package executor
4+
5+import (
6+ "archive/tar"
7+ "bytes"
8+ "compress/gzip"
9+ "os"
10+ "path/filepath"
11+ "strings"
12+ "testing"
13+)
14+
15+type tent struct {
16+ name, body, link string
17+ typ byte
18+}
19+
20+func tarball(t *testing.T, ents ...tent) *bytes.Reader {
21+ t.Helper()
22+ var buf bytes.Buffer
23+ zw := gzip.NewWriter(&buf)
24+ tw := tar.NewWriter(zw)
25+ for _, e := range ents {
26+ typ := e.typ
27+ if typ == 0 {
28+ typ = tar.TypeReg
29+ }
30+ h := &tar.Header{Name: e.name, Typeflag: typ, Mode: 0o644, Linkname: e.link}
31+ if typ == tar.TypeReg {
32+ h.Size = int64(len(e.body))
33+ }
34+ if typ == tar.TypeDir {
35+ h.Mode = 0o755
36+ }
37+ if err := tw.WriteHeader(h); err != nil {
38+ t.Fatal(err)
39+ }
40+ if typ == tar.TypeReg {
41+ _, _ = tw.Write([]byte(e.body))
42+ }
43+ }
44+ _ = tw.Close()
45+ _ = zw.Close()
46+ return bytes.NewReader(buf.Bytes())
47+}
48+
49+func TestExtractTarball(t *testing.T) {
50+ dir := t.TempDir()
51+ err := extractTarball(tarball(t,
52+ 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/sub/main.js", body: "1"},
55+ tent{name: "o-r-abc/link", typ: tar.TypeSymlink, link: "sub/main.js"},
56+ ), dir, nil)
57+ if err != nil {
58+ t.Fatal(err)
59+ }
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+ }
63+ if b, _ := os.ReadFile(filepath.Join(dir, "o-r-abc", "link")); string(b) != "1" {
64+ t.Errorf("link: %q", b)
65+ }
66+}
67+
68+func TestExtractTarballRejects(t *testing.T) {
69+ 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+ }
79+ for name, ents := range cases {
80+ t.Run(name, func(t *testing.T) {
81+ parent := t.TempDir()
82+ dir := filepath.Join(parent, "dest")
83+ if err := os.Mkdir(dir, 0o755); err != nil {
84+ t.Fatal(err)
85+ }
86+ err := extractTarball(tarball(t, ents...), dir, nil)
87+ if err == nil {
88+ t.Fatal("accepted")
89+ }
90+ if _, statErr := os.Lstat(filepath.Join(parent, "x")); statErr == nil {
91+ t.Fatal("wrote outside")
92+ }
93+ })
94+ }
95+}
96+
97+func TestExtractTarballNotGzip(t *testing.T) {
98+ err := extractTarball(strings.NewReader("not gzip"), t.TempDir(), nil)
99+ if err == nil || !strings.Contains(err.Error(), "not a gzip") {
100+ t.Fatalf("%v", err)
101+ }
102+}
103+
104+// A symlink to a directory inside the tree must not let a later entry write
105+// outside it, even through a chain of links.
106+func TestExtractTarballLinkChain(t *testing.T) {
107+ parent := t.TempDir()
108+ dir := filepath.Join(parent, "dest")
109+ _ = 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"},
114+ ), dir, nil)
115+ if err != nil {
116+ t.Fatal(err)
117+ }
118+ if b, _ := os.ReadFile(filepath.Join(dir, "f")); string(b) != "x" {
119+ t.Fatalf("f: %q", b)
120+ }
121+}
@@ -0,0 +1,121 @@
1+// SPDX-License-Identifier: MIT
2+
3+package executor
4+
5+import (
6+ "archive/tar"
7+ "bytes"
8+ "compress/gzip"
9+ "os"
10+ "path/filepath"
11+ "strings"
12+ "testing"
13+)
14+
15+type tent struct {
16+ name, body, link string
17+ typ byte
18+}
19+
20+func tarball(t *testing.T, ents ...tent) *bytes.Reader {
21+ t.Helper()
22+ var buf bytes.Buffer
23+ zw := gzip.NewWriter(&buf)
24+ tw := tar.NewWriter(zw)
25+ for _, e := range ents {
26+ typ := e.typ
27+ if typ == 0 {
28+ typ = tar.TypeReg
29+ }
30+ h := &tar.Header{Name: e.name, Typeflag: typ, Mode: 0o644, Linkname: e.link}
31+ if typ == tar.TypeReg {
32+ h.Size = int64(len(e.body))
33+ }
34+ if typ == tar.TypeDir {
35+ h.Mode = 0o755
36+ }
37+ if err := tw.WriteHeader(h); err != nil {
38+ t.Fatal(err)
39+ }
40+ if typ == tar.TypeReg {
41+ _, _ = tw.Write([]byte(e.body))
42+ }
43+ }
44+ _ = tw.Close()
45+ _ = zw.Close()
46+ return bytes.NewReader(buf.Bytes())
47+}
48+
49+func TestExtractTarball(t *testing.T) {
50+ dir := t.TempDir()
51+ err := extractTarball(tarball(t,
52+ 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/sub/main.js", body: "1"},
55+ tent{name: "o-r-abc/link", typ: tar.TypeSymlink, link: "sub/main.js"},
56+ ), dir, nil)
57+ if err != nil {
58+ t.Fatal(err)
59+ }
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+ }
63+ if b, _ := os.ReadFile(filepath.Join(dir, "o-r-abc", "link")); string(b) != "1" {
64+ t.Errorf("link: %q", b)
65+ }
66+}
67+
68+func TestExtractTarballRejects(t *testing.T) {
69+ 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+ }
79+ for name, ents := range cases {
80+ t.Run(name, func(t *testing.T) {
81+ parent := t.TempDir()
82+ dir := filepath.Join(parent, "dest")
83+ if err := os.Mkdir(dir, 0o755); err != nil {
84+ t.Fatal(err)
85+ }
86+ err := extractTarball(tarball(t, ents...), dir, nil)
87+ if err == nil {
88+ t.Fatal("accepted")
89+ }
90+ if _, statErr := os.Lstat(filepath.Join(parent, "x")); statErr == nil {
91+ t.Fatal("wrote outside")
92+ }
93+ })
94+ }
95+}
96+
97+func TestExtractTarballNotGzip(t *testing.T) {
98+ err := extractTarball(strings.NewReader("not gzip"), t.TempDir(), nil)
99+ if err == nil || !strings.Contains(err.Error(), "not a gzip") {
100+ t.Fatalf("%v", err)
101+ }
102+}
103+
104+// A symlink to a directory inside the tree must not let a later entry write
105+// outside it, even through a chain of links.
106+func TestExtractTarballLinkChain(t *testing.T) {
107+ parent := t.TempDir()
108+ dir := filepath.Join(parent, "dest")
109+ _ = 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"},
114+ ), dir, nil)
115+ if err != nil {
116+ t.Fatal(err)
117+ }
118+ if b, _ := os.ReadFile(filepath.Join(dir, "f")); string(b) != "x" {
119+ t.Fatalf("f: %q", b)
120+ }
121+}

Keyboard shortcuts

Single-key shortcuts are on. ctrl k and esc work whatever this says.

ctrl k :
go to a section, branch, tag, file or repository
?
this list
j k
move the row cursor down / up
enter
open the row under the cursor; on Files, a directory opens or closes in place
↑ ↓ ← → home end
in the file tree (focus on a row): previous / next row, close / open a directory or go to its parent / first entry, first / last row. They never open a file
l h
on Files: open a directory or a file / close a directory or go up to its parent, then to the parent page. Elsewhere: into the side panel (on a merge request, its files) / back to the content; there j k move between its controls
g g G
first / last row
/
filter the list; on Files, find a file anywhere in the repository (esc clears, enter opens the best match)
b
switch branch or tag
g s g f g c g b g m g i g r g a
tabs: summary, files, commits, branches & tags, merge requests, issues, releases, actions (when available)
g h
home
w
wide mode: use the whole width of the screen, or not (remembered)
y
copy a permalink pinned to the commit
d u x
diff: split / unified / collapse or expand all
e E
diff: the whole file in view / every file, with the lines the hunks hide
n
merge request: next note in the diff
esc
close a dialog, leave a field

Branches 4

Tags 11

↑↓ moveenter openesc close