Skip to content

Commit 693053b

Browse files
fsabiufsabiu
committed
Constrain tar driver paths to image root
1 parent 875db30 commit 693053b

5 files changed

Lines changed: 382 additions & 12 deletions

File tree

‎internal/pkgutil/root_path.go‎

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
package pkgutil
2+
3+
import (
4+
"fmt"
5+
"os"
6+
pathpkg "path"
7+
"path/filepath"
8+
"strings"
9+
)
10+
11+
const maxRootPathSymlinks = 255
12+
13+
// ResolvePathInRoot resolves imagePath below root. Absolute paths are treated as
14+
// absolute within root, and symlink targets are resolved using the same image
15+
// root instead of the host root.
16+
func ResolvePathInRoot(root, imagePath string, followFinalSymlink bool) (string, error) {
17+
absRoot, err := filepath.Abs(root)
18+
if err != nil {
19+
return "", err
20+
}
21+
absRoot = filepath.Clean(absRoot)
22+
23+
parts, err := resolveRootPathParts(nil, imagePath)
24+
if err != nil {
25+
return "", err
26+
}
27+
28+
resolved := []string{}
29+
symlinkCount := 0
30+
for i := 0; i < len(parts); i++ {
31+
part := parts[i]
32+
candidate := joinRootPath(absRoot, append(resolved, part))
33+
isFinal := i == len(parts)-1
34+
if !isFinal || followFinalSymlink {
35+
info, err := os.Lstat(candidate)
36+
if err != nil && !os.IsNotExist(err) {
37+
return "", err
38+
}
39+
if err == nil && info.Mode()&os.ModeSymlink != 0 {
40+
symlinkCount++
41+
if symlinkCount > maxRootPathSymlinks {
42+
return "", fmt.Errorf("too many symlinks resolving path %q", imagePath)
43+
}
44+
linkTarget, err := os.Readlink(candidate)
45+
if err != nil {
46+
return "", err
47+
}
48+
base := resolved
49+
if pathpkg.IsAbs(filepath.ToSlash(linkTarget)) {
50+
base = nil
51+
}
52+
linkParts, err := resolveRootPathParts(base, linkTarget)
53+
if err != nil {
54+
return "", err
55+
}
56+
parts = append(linkParts, parts[i+1:]...)
57+
resolved = nil
58+
i = -1
59+
continue
60+
}
61+
}
62+
resolved = append(resolved, part)
63+
}
64+
return joinRootPath(absRoot, resolved), nil
65+
}
66+
67+
func resolveRootPathParts(base []string, imagePath string) ([]string, error) {
68+
cleaned := filepath.ToSlash(imagePath)
69+
if pathpkg.IsAbs(cleaned) {
70+
base = nil
71+
}
72+
73+
parts := append([]string{}, base...)
74+
for _, part := range strings.Split(pathpkg.Clean(cleaned), "/") {
75+
switch part {
76+
case "", ".":
77+
continue
78+
case "..":
79+
if len(parts) == 0 {
80+
return nil, fmt.Errorf("path %q escapes root", imagePath)
81+
}
82+
parts = parts[:len(parts)-1]
83+
default:
84+
parts = append(parts, part)
85+
}
86+
}
87+
return parts, nil
88+
}
89+
90+
func joinRootPath(root string, parts []string) string {
91+
return filepath.Join(append([]string{root}, parts...)...)
92+
}

‎internal/pkgutil/tar_utils.go‎

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"fmt"
2222
"io"
2323
"os"
24+
pathpkg "path"
2425
"path/filepath"
2526
"strings"
2627

@@ -50,7 +51,10 @@ func unpackTar(tr *tar.Reader, path string, whitelist []string) error {
5051
if err != nil {
5152
return errors.Wrap(err, "Error getting next tar header")
5253
}
53-
target := filepath.Clean(filepath.Join(path, header.Name))
54+
target, err := ResolvePathInRoot(path, header.Name, false)
55+
if err != nil {
56+
return errors.Wrap(err, "resolving tar entry path")
57+
}
5458
// Make sure the target isn't part of the whitelist
5559
if checkWhitelist(target, whitelist) {
5660
continue
@@ -60,7 +64,7 @@ func unpackTar(tr *tar.Reader, path string, whitelist []string) error {
6064

6165
// if its a dir and it doesn't exist create it
6266
case tar.TypeDir:
63-
if _, err := os.Stat(target); os.IsNotExist(err) {
67+
if _, err := os.Lstat(target); os.IsNotExist(err) {
6468
if mode.Perm()&(1<<(uint(7))) == 0 {
6569
logrus.Debugf("Write permission bit not set on %s by default; setting manually", target)
6670
originalMode := mode
@@ -93,7 +97,7 @@ func unpackTar(tr *tar.Reader, path string, whitelist []string) error {
9397
}
9498
// It's possible we end up creating files that can't be overwritten based on their permissions.
9599
// Explicitly delete an existing file before continuing.
96-
if _, err := os.Stat(target); !os.IsNotExist(err) {
100+
if _, err := os.Lstat(target); !os.IsNotExist(err) {
97101
logrus.Debugf("Removing %s for overwrite", target)
98102
if err := os.Remove(target); err != nil {
99103
logrus.Errorf("error removing file %s", target)
@@ -118,9 +122,12 @@ func unpackTar(tr *tar.Reader, path string, whitelist []string) error {
118122
}
119123
currFile.Close()
120124
case tar.TypeSymlink:
125+
if _, err := ResolvePathInRoot(path, tarLinkTarget(header.Name, header.Linkname), true); err != nil {
126+
return errors.Wrap(err, "resolving tar symlink path")
127+
}
121128
// It's possible we end up creating files that can't be overwritten based on their permissions.
122129
// Explicitly delete an existing file before continuing.
123-
if _, err := os.Stat(target); !os.IsNotExist(err) {
130+
if _, err := os.Lstat(target); !os.IsNotExist(err) {
124131
logrus.Debugf("Removing %s to create symlink", target)
125132
if err := os.RemoveAll(target); err != nil {
126133
logrus.Debugf("Unable to remove %s: %s", target, err)
@@ -131,7 +138,10 @@ func unpackTar(tr *tar.Reader, path string, whitelist []string) error {
131138
logrus.Errorf("Failed to create symlink between %s and %s: %s", header.Linkname, target, err)
132139
}
133140
case tar.TypeLink:
134-
linkname := filepath.Clean(filepath.Join(path, header.Linkname))
141+
linkname, err := ResolvePathInRoot(path, header.Linkname, true)
142+
if err != nil {
143+
return errors.Wrap(err, "resolving tar hard link path")
144+
}
135145
// Check if the linkname already exists
136146
if _, err := os.Stat(linkname); !os.IsNotExist(err) {
137147
// If it exists, create the hard link
@@ -168,6 +178,13 @@ func unpackTar(tr *tar.Reader, path string, whitelist []string) error {
168178
return nil
169179
}
170180

181+
func tarLinkTarget(name, linkname string) string {
182+
if pathpkg.IsAbs(filepath.ToSlash(linkname)) {
183+
return linkname
184+
}
185+
return pathpkg.Join(pathpkg.Dir(filepath.ToSlash(name)), filepath.ToSlash(linkname))
186+
}
187+
171188
func resolveHardlink(linkname, target string) error {
172189
if err := os.Link(linkname, target); err != nil {
173190
return err

‎internal/pkgutil/tar_utils_test.go‎

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
1+
package pkgutil
2+
3+
import (
4+
"archive/tar"
5+
"bytes"
6+
"os"
7+
"path/filepath"
8+
"testing"
9+
)
10+
11+
func testTarReader(t *testing.T, entries ...func(*tar.Writer) error) *tar.Reader {
12+
t.Helper()
13+
14+
var buf bytes.Buffer
15+
tw := tar.NewWriter(&buf)
16+
for _, entry := range entries {
17+
if err := entry(tw); err != nil {
18+
t.Fatal(err)
19+
}
20+
}
21+
if err := tw.Close(); err != nil {
22+
t.Fatal(err)
23+
}
24+
return tar.NewReader(bytes.NewReader(buf.Bytes()))
25+
}
26+
27+
func testTarFile(name, body string) func(*tar.Writer) error {
28+
return func(tw *tar.Writer) error {
29+
if err := tw.WriteHeader(&tar.Header{
30+
Name: name,
31+
Mode: 0644,
32+
Size: int64(len(body)),
33+
}); err != nil {
34+
return err
35+
}
36+
_, err := tw.Write([]byte(body))
37+
return err
38+
}
39+
}
40+
41+
func testTarDir(name string) func(*tar.Writer) error {
42+
return func(tw *tar.Writer) error {
43+
return tw.WriteHeader(&tar.Header{
44+
Name: name,
45+
Mode: 0755,
46+
Typeflag: tar.TypeDir,
47+
})
48+
}
49+
}
50+
51+
func testTarSymlink(name, linkname string) func(*tar.Writer) error {
52+
return func(tw *tar.Writer) error {
53+
return tw.WriteHeader(&tar.Header{
54+
Name: name,
55+
Linkname: linkname,
56+
Typeflag: tar.TypeSymlink,
57+
})
58+
}
59+
}
60+
61+
func testTarHardlink(name, linkname string) func(*tar.Writer) error {
62+
return func(tw *tar.Writer) error {
63+
return tw.WriteHeader(&tar.Header{
64+
Name: name,
65+
Linkname: linkname,
66+
Typeflag: tar.TypeLink,
67+
})
68+
}
69+
}
70+
71+
func TestUnpackTarRejectsEntriesOutsideRoot(t *testing.T) {
72+
parent := t.TempDir()
73+
root := filepath.Join(parent, "root")
74+
if err := os.MkdirAll(root, 0755); err != nil {
75+
t.Fatal(err)
76+
}
77+
78+
err := unpackTar(testTarReader(t, testTarFile("../outside.txt", "outside-data")), root, nil)
79+
if err == nil {
80+
t.Fatal("unpackTar allowed an entry outside the root")
81+
}
82+
if _, err := os.Stat(filepath.Join(parent, "outside.txt")); !os.IsNotExist(err) {
83+
t.Fatalf("outside path exists after rejected unpack: %v", err)
84+
}
85+
}
86+
87+
func TestUnpackTarRejectsSymlinksOutsideRoot(t *testing.T) {
88+
parent := t.TempDir()
89+
root := filepath.Join(parent, "root")
90+
if err := os.MkdirAll(root, 0755); err != nil {
91+
t.Fatal(err)
92+
}
93+
94+
err := unpackTar(testTarReader(t, testTarSymlink("link", "..")), root, nil)
95+
if err == nil {
96+
t.Fatal("unpackTar allowed a symlink outside the root")
97+
}
98+
}
99+
100+
func TestUnpackTarRejectsHardlinksOutsideRoot(t *testing.T) {
101+
root := t.TempDir()
102+
103+
err := unpackTar(testTarReader(t, testTarHardlink("link", "../outside.txt")), root, nil)
104+
if err == nil {
105+
t.Fatal("unpackTar allowed a hard link outside the root")
106+
}
107+
}
108+
109+
func TestUnpackTarResolvesSymlinksWithinRoot(t *testing.T) {
110+
root := t.TempDir()
111+
112+
err := unpackTar(testTarReader(t,
113+
testTarDir("etc"),
114+
testTarSymlink("link", "/etc"),
115+
testTarFile("link/generated.txt", "image-data"),
116+
), root, nil)
117+
if err != nil {
118+
t.Fatal(err)
119+
}
120+
121+
got, err := os.ReadFile(filepath.Join(root, "etc", "generated.txt"))
122+
if err != nil {
123+
t.Fatal(err)
124+
}
125+
if string(got) != "image-data" {
126+
t.Fatalf("unpacked file through image symlink = %q", got)
127+
}
128+
}

‎pkg/drivers/tar_driver.go‎

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ package drivers
1717
import (
1818
"io/fs"
1919
"os"
20-
"path/filepath"
2120
"strings"
2221

2322
"github.com/pkg/errors"
@@ -138,16 +137,28 @@ func (d *TarDriver) ProcessCommand(_ []unversioned.EnvVar, _ []string) (string,
138137
return "", "", -1, errors.New("Tar driver is unable to process commands, please use a different driver")
139138
}
140139

141-
func (d *TarDriver) StatFile(path string) (os.FileInfo, error) {
142-
return os.Lstat(filepath.Join(d.Image.FSPath, path))
140+
func (d *TarDriver) StatFile(imagePath string) (os.FileInfo, error) {
141+
path, err := pkgutil.ResolvePathInRoot(d.Image.FSPath, imagePath, false)
142+
if err != nil {
143+
return nil, err
144+
}
145+
return os.Lstat(path)
143146
}
144147

145-
func (d *TarDriver) ReadFile(path string) ([]byte, error) {
146-
return os.ReadFile(filepath.Join(d.Image.FSPath, path))
148+
func (d *TarDriver) ReadFile(imagePath string) ([]byte, error) {
149+
path, err := pkgutil.ResolvePathInRoot(d.Image.FSPath, imagePath, true)
150+
if err != nil {
151+
return nil, err
152+
}
153+
return os.ReadFile(path)
147154
}
148155

149-
func (d *TarDriver) ReadDir(path string) ([]os.FileInfo, error) {
150-
entries, err := os.ReadDir(filepath.Join(d.Image.FSPath, path))
156+
func (d *TarDriver) ReadDir(imagePath string) ([]os.FileInfo, error) {
157+
path, err := pkgutil.ResolvePathInRoot(d.Image.FSPath, imagePath, true)
158+
if err != nil {
159+
return nil, err
160+
}
161+
entries, err := os.ReadDir(path)
151162
if err != nil {
152163
return nil, err
153164
}

0 commit comments

Comments
 (0)