Skip to content

Commit f5a96e1

Browse files
fs: Scope all DirFS operations through os.Root (#2187)
This runs all filesystem operations of DirFS through an `os.Root` instance to prevent any kind of path-traversal outside of that root, either through walking the paths directly or through following symlinks. Behavior has largerly been preserved. Notable mentions are: - `Mknode`: There's no `Mknodat` for non-linux targets but this likely was the effective behavior beforehand as well. We used to gracefully handle errors by writing empty files. That behavior is preserved across platforms. - `Chmod`/`Chown`: We used to ignore all errors due to filesystem incompatibilities and the presence of the in-memory overlay. We're now only ignoring errors that point to those incompatibilities and are surfacing any other errors to the callers.
1 parent 503b545 commit f5a96e1

5 files changed

Lines changed: 766 additions & 255 deletions

File tree

pkg/apk/apk/path_traversal_test.go

Lines changed: 251 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,257 @@ func TestPathTraversalHardlink(t *testing.T) {
8888
}
8989
}
9090

91+
// TestSymlinkEscape_FileThroughAbsoluteSymlink covers the classic symlink-escape
92+
// shape: a malicious APK plants a symlink inside the image whose target is an
93+
// absolute path pointing outside the rootfs, then a regular file whose tar
94+
// header name traverses that symlink. The install must fail and the outside
95+
// path must remain untouched.
96+
func TestSymlinkEscape_FileThroughAbsoluteSymlink(t *testing.T) {
97+
ctx := t.Context()
98+
99+
sandbox := t.TempDir()
100+
base := filepath.Join(sandbox, "base")
101+
outsideDir := filepath.Join(sandbox, "outside")
102+
outsideFile := filepath.Join(outsideDir, "pwned")
103+
if err := os.MkdirAll(outsideDir, 0o755); err != nil {
104+
t.Fatal(err)
105+
}
106+
107+
fsys := apkfs.DirFS(ctx, base, apkfs.WithCreateDir())
108+
if fsys == nil {
109+
t.Fatalf("failed to create dirfs for base %s", base)
110+
}
111+
112+
a, err := New(ctx, WithFS(fsys))
113+
if err != nil {
114+
t.Fatalf("apk.New: %v", err)
115+
}
116+
117+
r, err := makeSymlinkThenFileTar("evil", outsideDir, "evil/pwned", []byte("malicious"))
118+
if err != nil {
119+
t.Fatalf("makeSymlinkThenFileTar: %v", err)
120+
}
121+
122+
if _, err := a.installAPKFiles(ctx, r, &Package{}); err == nil {
123+
t.Fatalf("expected installAPKFiles to fail, but it succeeded")
124+
}
125+
126+
if _, statErr := os.Stat(outsideFile); statErr == nil {
127+
t.Fatalf("expected %s to not exist after fix", outsideFile)
128+
}
129+
}
130+
131+
// TestSymlinkEscape_FileThroughRelativeSymlink is the ../outside variant.
132+
func TestSymlinkEscape_FileThroughRelativeSymlink(t *testing.T) {
133+
ctx := t.Context()
134+
135+
sandbox := t.TempDir()
136+
base := filepath.Join(sandbox, "base")
137+
outsideDir := filepath.Join(sandbox, "outside")
138+
outsideFile := filepath.Join(outsideDir, "pwned")
139+
if err := os.MkdirAll(outsideDir, 0o755); err != nil {
140+
t.Fatal(err)
141+
}
142+
143+
fsys := apkfs.DirFS(ctx, base, apkfs.WithCreateDir())
144+
if fsys == nil {
145+
t.Fatalf("failed to create dirfs for base %s", base)
146+
}
147+
148+
a, err := New(ctx, WithFS(fsys))
149+
if err != nil {
150+
t.Fatalf("apk.New: %v", err)
151+
}
152+
153+
r, err := makeSymlinkThenFileTar("evil", "../outside", "evil/pwned", []byte("malicious"))
154+
if err != nil {
155+
t.Fatalf("makeSymlinkThenFileTar: %v", err)
156+
}
157+
158+
if _, err := a.installAPKFiles(ctx, r, &Package{}); err == nil {
159+
t.Fatalf("expected installAPKFiles to fail, but it succeeded")
160+
}
161+
162+
if _, statErr := os.Stat(outsideFile); statErr == nil {
163+
t.Fatalf("expected %s to not exist after fix", outsideFile)
164+
}
165+
}
166+
167+
// TestSymlinkEscape_MkdirAllThroughSymlink plants a symlink whose target is an
168+
// outside directory, then a TypeDir entry traversing the symlink. MkdirAll
169+
// must not merge new dirs into the outside location.
170+
func TestSymlinkEscape_MkdirAllThroughSymlink(t *testing.T) {
171+
ctx := t.Context()
172+
173+
sandbox := t.TempDir()
174+
base := filepath.Join(sandbox, "base")
175+
outsideDir := filepath.Join(sandbox, "outside")
176+
outsideSub := filepath.Join(outsideDir, "sub")
177+
if err := os.MkdirAll(outsideDir, 0o755); err != nil {
178+
t.Fatal(err)
179+
}
180+
181+
fsys := apkfs.DirFS(ctx, base, apkfs.WithCreateDir())
182+
if fsys == nil {
183+
t.Fatalf("failed to create dirfs for base %s", base)
184+
}
185+
186+
a, err := New(ctx, WithFS(fsys))
187+
if err != nil {
188+
t.Fatalf("apk.New: %v", err)
189+
}
190+
191+
r, err := makeSymlinkThenDirTar("evil", outsideDir, "evil/sub")
192+
if err != nil {
193+
t.Fatalf("makeSymlinkThenDirTar: %v", err)
194+
}
195+
196+
if _, err := a.installAPKFiles(ctx, r, &Package{}); err == nil {
197+
t.Fatalf("expected installAPKFiles to fail, but it succeeded")
198+
}
199+
200+
if _, statErr := os.Stat(outsideSub); statErr == nil {
201+
t.Fatalf("expected %s to not exist after fix", outsideSub)
202+
}
203+
}
204+
205+
// TestSymlinkEscape_HardlinkThroughSymlink validates the hardlink path: the
206+
// prior GHSA guarded target-side escapes, but the newname side could still be
207+
// redirected through an attacker-planted symlink.
208+
func TestSymlinkEscape_HardlinkThroughSymlink(t *testing.T) {
209+
ctx := t.Context()
210+
211+
sandbox := t.TempDir()
212+
base := filepath.Join(sandbox, "base")
213+
outsideDir := filepath.Join(sandbox, "outside")
214+
outsideLinked := filepath.Join(outsideDir, "linked")
215+
if err := os.MkdirAll(outsideDir, 0o755); err != nil {
216+
t.Fatal(err)
217+
}
218+
219+
fsys := apkfs.DirFS(ctx, base, apkfs.WithCreateDir())
220+
if fsys == nil {
221+
t.Fatalf("failed to create dirfs for base %s", base)
222+
}
223+
224+
a, err := New(ctx, WithFS(fsys))
225+
if err != nil {
226+
t.Fatalf("apk.New: %v", err)
227+
}
228+
229+
r, err := makeHardlinkThroughSymlinkTar("legit", "evil", outsideDir, "evil/linked")
230+
if err != nil {
231+
t.Fatalf("makeHardlinkThroughSymlinkTar: %v", err)
232+
}
233+
234+
if _, err := a.installAPKFiles(ctx, r, &Package{}); err == nil {
235+
t.Fatalf("expected installAPKFiles to fail, but it succeeded")
236+
}
237+
238+
if _, statErr := os.Lstat(outsideLinked); statErr == nil {
239+
t.Fatalf("expected %s to not exist after fix", outsideLinked)
240+
}
241+
}
242+
243+
func makeSymlinkThenFileTar(symlinkName, symlinkTarget, fileName string, content []byte) (*bytes.Reader, error) {
244+
var buf bytes.Buffer
245+
tw := tar.NewWriter(&buf)
246+
247+
if err := tw.WriteHeader(&tar.Header{
248+
Name: symlinkName,
249+
Linkname: symlinkTarget,
250+
Typeflag: tar.TypeSymlink,
251+
Mode: 0o777,
252+
}); err != nil {
253+
return nil, err
254+
}
255+
256+
if err := tw.WriteHeader(&tar.Header{
257+
Name: fileName,
258+
Typeflag: tar.TypeReg,
259+
Mode: 0o644,
260+
Size: int64(len(content)),
261+
}); err != nil {
262+
return nil, err
263+
}
264+
if _, err := tw.Write(content); err != nil {
265+
return nil, err
266+
}
267+
268+
if err := tw.Close(); err != nil {
269+
return nil, err
270+
}
271+
return bytes.NewReader(buf.Bytes()), nil
272+
}
273+
274+
func makeSymlinkThenDirTar(symlinkName, symlinkTarget, dirName string) (*bytes.Reader, error) {
275+
var buf bytes.Buffer
276+
tw := tar.NewWriter(&buf)
277+
278+
if err := tw.WriteHeader(&tar.Header{
279+
Name: symlinkName,
280+
Linkname: symlinkTarget,
281+
Typeflag: tar.TypeSymlink,
282+
Mode: 0o777,
283+
}); err != nil {
284+
return nil, err
285+
}
286+
287+
if err := tw.WriteHeader(&tar.Header{
288+
Name: dirName,
289+
Typeflag: tar.TypeDir,
290+
Mode: 0o755,
291+
}); err != nil {
292+
return nil, err
293+
}
294+
295+
if err := tw.Close(); err != nil {
296+
return nil, err
297+
}
298+
return bytes.NewReader(buf.Bytes()), nil
299+
}
300+
301+
func makeHardlinkThroughSymlinkTar(regularName, symlinkName, symlinkTarget, hardlinkName string) (*bytes.Reader, error) {
302+
var buf bytes.Buffer
303+
tw := tar.NewWriter(&buf)
304+
305+
content := []byte("legitimate")
306+
if err := tw.WriteHeader(&tar.Header{
307+
Name: regularName,
308+
Typeflag: tar.TypeReg,
309+
Mode: 0o644,
310+
Size: int64(len(content)),
311+
}); err != nil {
312+
return nil, err
313+
}
314+
if _, err := tw.Write(content); err != nil {
315+
return nil, err
316+
}
317+
318+
if err := tw.WriteHeader(&tar.Header{
319+
Name: symlinkName,
320+
Linkname: symlinkTarget,
321+
Typeflag: tar.TypeSymlink,
322+
Mode: 0o777,
323+
}); err != nil {
324+
return nil, err
325+
}
326+
327+
if err := tw.WriteHeader(&tar.Header{
328+
Name: hardlinkName,
329+
Linkname: regularName,
330+
Typeflag: tar.TypeLink,
331+
Mode: 0o644,
332+
}); err != nil {
333+
return nil, err
334+
}
335+
336+
if err := tw.Close(); err != nil {
337+
return nil, err
338+
}
339+
return bytes.NewReader(buf.Bytes()), nil
340+
}
341+
91342
func makeTestTar(dirName, symlinkName, symlinkTarget string) (*bytes.Reader, error) {
92343
var buf bytes.Buffer
93344
tw := tar.NewWriter(&buf)

0 commit comments

Comments
 (0)