Skip to content

Commit a98323c

Browse files
bussyjdOisinKyne
authored andcommitted
security(stackbackup): block symlink-target escape and decompression bombs on import
obol stack import sanitized tar entry NAMES (sanitizeEntryName) but wrote symlinks from the raw, unchecked Linkname. A malicious archive could ship a clean-named symlink whose target points outside the extraction root (e.g. link -> /etc, or ../../..), then a follow-up entry written THROUGH that link lands at an arbitrary path — arbitrary file write as the importing user. The only guard (name sanitization) does not help: the escape happens at OS symlink-resolution time, not lexically. - symlinkEscapesRoot rejects absolute and ..-walking symlink targets that resolve outside destRoot; in-root relative links (the common case) still work (TestArchiveRoundTrip unchanged). - Add a decompression-ratio guard (countingReader + ratioGuard between gzip and tar): aborts when uncompressed:compressed exceeds 100:1 past a 64 MiB floor, so a tiny gzip can no longer inflate to a disk-filling tar. The floor keeps legitimate multi-GB agent-data restores (low, steady ratio) passing; thresholds are vars so tests can lower them. - Tests: TestExtractRejectsSymlinkEscape (relative + absolute targets, no link created) and TestExtractRejectsDecompressionBomb. Found in review of #624.
1 parent aed9156 commit a98323c

3 files changed

Lines changed: 140 additions & 2 deletions

File tree

internal/stackbackup/import.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -137,12 +137,13 @@ func walkArchive(input string, fn func(*tar.Reader, *tar.Header, string) error)
137137
return err
138138
}
139139
defer f.Close()
140-
gz, err := gzip.NewReader(f)
140+
counted := &countingReader{r: f}
141+
gz, err := gzip.NewReader(counted)
141142
if err != nil {
142143
return fmt.Errorf("not a gzip archive: %w", err)
143144
}
144145
defer gz.Close()
145-
tr := tar.NewReader(gz)
146+
tr := tar.NewReader(&ratioGuard{r: gz, compressed: &counted.n})
146147
for {
147148
hdr, err := tr.Next()
148149
if errors.Is(err, io.EOF) {

internal/stackbackup/stackbackup_test.go

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,75 @@ func TestExtractRejectsPathTraversal(t *testing.T) {
234234
}
235235
}
236236

237+
// TestExtractRejectsSymlinkEscape covers the symlink variant of the traversal
238+
// escape: a clean entry NAME whose symlink TARGET points outside the root.
239+
// Without target validation, a follow-up entry written through the link would
240+
// land at an arbitrary path. Both a "../"-walking and an absolute target must
241+
// be refused, and no link may be created.
242+
func TestExtractRejectsSymlinkEscape(t *testing.T) {
243+
for _, target := range []string{"../../../../etc/cron.d", "/etc/passwd"} {
244+
archive := filepath.Join(t.TempDir(), "evil-symlink.tar.gz")
245+
f, err := os.Create(archive)
246+
if err != nil {
247+
t.Fatal(err)
248+
}
249+
gz := gzip.NewWriter(f)
250+
tw := tar.NewWriter(gz)
251+
if err := tw.WriteHeader(&tar.Header{Name: "link", Linkname: target, Mode: 0o777, Typeflag: tar.TypeSymlink}); err != nil {
252+
t.Fatal(err)
253+
}
254+
tw.Close()
255+
gz.Close()
256+
f.Close()
257+
258+
dest := t.TempDir()
259+
err = walkArchive(archive, func(tr *tar.Reader, hdr *tar.Header, clean string) error {
260+
return extractEntry(tr, hdr, dest, clean)
261+
})
262+
if err == nil || !strings.Contains(err.Error(), "escapes extraction root") {
263+
t.Fatalf("symlink escape to %q not rejected: %v", target, err)
264+
}
265+
if _, err := os.Lstat(filepath.Join(dest, "link")); !os.IsNotExist(err) {
266+
t.Fatalf("escaping symlink to %q must not be created: %v", target, err)
267+
}
268+
}
269+
}
270+
271+
// TestExtractRejectsDecompressionBomb verifies the ratio guard trips on an
272+
// archive that inflates far beyond its compressed size. Thresholds are lowered
273+
// so the test stays small and fast.
274+
func TestExtractRejectsDecompressionBomb(t *testing.T) {
275+
origFloor, origRatio := bombFloorBytes, maxCompressionRatio
276+
bombFloorBytes, maxCompressionRatio = 1024, 2
277+
defer func() { bombFloorBytes, maxCompressionRatio = origFloor, origRatio }()
278+
279+
archive := filepath.Join(t.TempDir(), "bomb.tar.gz")
280+
f, err := os.Create(archive)
281+
if err != nil {
282+
t.Fatal(err)
283+
}
284+
gz := gzip.NewWriter(f)
285+
tw := tar.NewWriter(gz)
286+
zeros := make([]byte, 1<<20) // 1 MiB of zeros gzips to ~1 KiB -> ~1000:1
287+
if err := tw.WriteHeader(&tar.Header{Name: "big", Mode: 0o600, Size: int64(len(zeros)), Typeflag: tar.TypeReg}); err != nil {
288+
t.Fatal(err)
289+
}
290+
if _, err := tw.Write(zeros); err != nil {
291+
t.Fatal(err)
292+
}
293+
tw.Close()
294+
gz.Close()
295+
f.Close()
296+
297+
dest := t.TempDir()
298+
err = walkArchive(archive, func(tr *tar.Reader, hdr *tar.Header, clean string) error {
299+
return extractEntry(tr, hdr, dest, clean)
300+
})
301+
if err == nil || !strings.Contains(err.Error(), "decompression bomb") {
302+
t.Fatalf("decompression bomb not rejected: %v", err)
303+
}
304+
}
305+
237306
func TestReadStackID(t *testing.T) {
238307
dir := t.TempDir()
239308
if got := readStackID(dir); got != "" {

internal/stackbackup/tar.go

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,13 +165,35 @@ func sanitizeEntryName(name string) (string, error) {
165165
return clean, nil
166166
}
167167

168+
// symlinkEscapesRoot reports whether a symlink placed at linkPath pointing to
169+
// target would resolve outside destRoot. Entry NAMES are sanitized
170+
// (sanitizeEntryName), but a symlink's TARGET is not — an unchecked target
171+
// lets a LATER entry be written THROUGH the link to an arbitrary path (the
172+
// classic symlink tar-extraction escape: entry "x" -> /etc, then entry
173+
// "x/passwd" resolves outside the root). Absolute targets and ".."-walking
174+
// relative targets that leave the root are rejected; in-root links (the common
175+
// case — a relative link to a sibling file) are allowed.
176+
func symlinkEscapesRoot(destRoot, linkPath, target string) bool {
177+
var resolved string
178+
if filepath.IsAbs(target) {
179+
resolved = filepath.Clean(target)
180+
} else {
181+
resolved = filepath.Clean(filepath.Join(filepath.Dir(linkPath), target))
182+
}
183+
root := filepath.Clean(destRoot)
184+
return resolved != root && !strings.HasPrefix(resolved, root+string(os.PathSeparator))
185+
}
186+
168187
// extractEntry writes one tar entry under destRoot/relName.
169188
func extractEntry(tr *tar.Reader, hdr *tar.Header, destRoot, relName string) error {
170189
dest := filepath.Join(destRoot, relName)
171190
switch hdr.Typeflag {
172191
case tar.TypeDir:
173192
return os.MkdirAll(dest, os.FileMode(hdr.Mode)|0o700)
174193
case tar.TypeSymlink:
194+
if symlinkEscapesRoot(destRoot, dest, hdr.Linkname) {
195+
return fmt.Errorf("symlink %q targets %q which escapes extraction root", relName, hdr.Linkname)
196+
}
175197
if err := os.MkdirAll(filepath.Dir(dest), 0o700); err != nil {
176198
return err
177199
}
@@ -194,3 +216,49 @@ func extractEntry(tr *tar.Reader, hdr *tar.Header, destRoot, relName string) err
194216
return nil // ignore other entry types
195217
}
196218
}
219+
220+
// Decompression-bomb guard. archive/tar bounds each file read to the header
221+
// size, but a tiny gzip can declare (and inflate to) a disk-filling tar. The
222+
// guard tracks the live ratio of uncompressed bytes produced to compressed
223+
// bytes consumed and aborts once it is implausible. bombFloorBytes avoids
224+
// false positives on small, naturally high-ratio inputs; legitimate multi-GB
225+
// agent-data exports never sustain a ratio this high. Both are vars so tests
226+
// can lower them. (vars, not consts, for that reason.)
227+
var (
228+
bombFloorBytes int64 = 64 << 20 // ignore the ratio below this many uncompressed bytes
229+
maxCompressionRatio int64 = 100 // abort above this uncompressed:compressed ratio
230+
)
231+
232+
// countingReader counts the bytes read from an underlying reader (used on the
233+
// raw compressed stream, beneath gzip).
234+
type countingReader struct {
235+
r io.Reader
236+
n int64
237+
}
238+
239+
func (c *countingReader) Read(p []byte) (int, error) {
240+
n, err := c.r.Read(p)
241+
c.n += int64(n)
242+
return n, err
243+
}
244+
245+
// ratioGuard wraps the decompressed stream and trips when the running
246+
// decompression ratio exceeds maxCompressionRatio past bombFloorBytes. It
247+
// sits between gzip and tar, so every byte the tar reader consumes — headers
248+
// and file bodies alike, including io.Copy in extractEntry — is accounted.
249+
type ratioGuard struct {
250+
r io.Reader
251+
compressed *int64 // bytes pulled from the compressed source so far
252+
uncompressed int64
253+
}
254+
255+
func (g *ratioGuard) Read(p []byte) (int, error) {
256+
n, err := g.r.Read(p)
257+
g.uncompressed += int64(n)
258+
if g.uncompressed > bombFloorBytes {
259+
if c := *g.compressed; c > 0 && g.uncompressed/c > maxCompressionRatio {
260+
return n, fmt.Errorf("refusing archive: decompression ratio %d:1 exceeds %d:1 limit (possible decompression bomb)", g.uncompressed/c, maxCompressionRatio)
261+
}
262+
}
263+
return n, err
264+
}

0 commit comments

Comments
 (0)