diff --git a/internal/parser/archive.go b/internal/parser/archive.go index d4dcef2..3f1a9da 100644 --- a/internal/parser/archive.go +++ b/internal/parser/archive.go @@ -18,7 +18,15 @@ import ( const maxSingleFileSize = 10 * 1024 * 1024 const maxSingleFileSizeLarge = 1024 * 1024 * 1024 const maxZipArchiveSize = 50 * 1024 * 1024 -const maxGzipDecompressedSize = 50 * 1024 * 1024 + +// A plain gzipped log file (e.g. nvidia-bug-report-*.log.gz) is decompressed +// fully into memory, but without a fixed byte cap — real bug-report dumps +// routinely exceed 50MB and a hard cap silently drops the tail. Instead it's +// bounded by a decompression-ratio guard (catches gzip bombs, which achieve +// enormous ratios) plus a generous absolute ceiling as a memory safety net. +const gzipBombRatio = 300 // decompressed:compressed ratio that trips the bomb guard +const gzipBombMinCompressed = 4096 // don't evaluate the ratio until this many compressed bytes have actually been consumed, to avoid false positives from gzip header/buffering overhead on tiny inputs +const gzipAbsoluteCeiling = 1024 * 1024 * 1024 // 1GB hard safety net regardless of ratio, matches maxSingleFileSizeLarge var supportedArchiveExt = map[string]struct{}{ @@ -167,7 +175,8 @@ func extractTarFromReader(r io.Reader) ([]ExtractedFile, error) { } func extractTarGzFromReader(r io.Reader, filename string) ([]ExtractedFile, error) { - gzr, err := gzip.NewReader(r) + compressed := &countingReader{r: r} + gzr, err := gzip.NewReader(compressed) if err != nil { return nil, fmt.Errorf("gzip reader: %w", err) } @@ -192,15 +201,12 @@ func extractTarGzFromReader(r io.Reader, filename string) ([]ExtractedFile, erro return extractTarEntriesWithLimit(buf, maxSingleFileSizeLarge) } - // Not a tar archive - treat as a single gzipped file. A hard byte cap - // is safe here since there's no container structure to corrupt. - decompressed, err := io.ReadAll(io.LimitReader(buf, maxGzipDecompressedSize+1)) + // Not a tar archive - treat as a single gzipped file. No fixed byte cap: + // stream the decompression and only stop early on a runaway compression + // ratio (gzip bomb) or the absolute memory safety net. + decompressed, truncMsg, err := readGzipWithBombGuard(buf, compressed) if err != nil { - return nil, fmt.Errorf("read gzip content: %w", err) - } - gzipTruncated := len(decompressed) > maxGzipDecompressedSize - if gzipTruncated { - decompressed = decompressed[:maxGzipDecompressedSize] + return nil, err } baseName := strings.TrimSuffix(filename, ".gz") @@ -213,17 +219,61 @@ func extractTarGzFromReader(r io.Reader, filename string) ([]ExtractedFile, erro Content: decompressed, ModTime: gzr.ModTime, } - if gzipTruncated { + if truncMsg != "" { file.Truncated = true - file.TruncatedMessage = fmt.Sprintf( - "decompressed gzip content exceeded %d bytes and was truncated", - maxGzipDecompressedSize, - ) + file.TruncatedMessage = truncMsg } return []ExtractedFile{file}, nil } +// countingReader tracks the number of bytes read from the underlying reader. +type countingReader struct { + r io.Reader + n int64 +} + +func (c *countingReader) Read(p []byte) (int, error) { + n, err := c.r.Read(p) + c.n += int64(n) + return n, err +} + +// readGzipWithBombGuard streams dec (a gzip decompression stream) to +// completion, tracking bytes consumed from the compressed source via +// compressed. It aborts early — returning what was decompressed so far plus +// a non-empty reason — if the decompression ratio suggests a gzip bomb, or +// if the absolute ceiling is hit regardless of ratio. +func readGzipWithBombGuard(dec io.Reader, compressed *countingReader) ([]byte, string, error) { + var out bytes.Buffer + chunk := make([]byte, 256*1024) + for { + n, readErr := dec.Read(chunk) + if n > 0 { + out.Write(chunk[:n]) + + if int64(out.Len()) > gzipAbsoluteCeiling { + return out.Bytes(), fmt.Sprintf( + "decompressed gzip content exceeded the %d byte safety ceiling and was truncated", + gzipAbsoluteCeiling, + ), nil + } + if compressed.n >= gzipBombMinCompressed && int64(out.Len()) > compressed.n*gzipBombRatio { + return out.Bytes(), fmt.Sprintf( + "decompression ratio exceeded %dx (likely a gzip bomb) and was truncated after %d bytes", + gzipBombRatio, out.Len(), + ), nil + } + } + if readErr == io.EOF { + return out.Bytes(), "", nil + } + if readErr != nil { + return nil, "", fmt.Errorf("read gzip content: %w", readErr) + } + } +} + // extractTarEntriesWithLimit streams tar entries from r, skipping individual // files over 10MB, and stops once the cumulative extracted content reaches // totalLimit — always at an entry boundary, never mid-file. diff --git a/internal/parser/archive_test.go b/internal/parser/archive_test.go index 4d9ceb9..3ac2b85 100644 --- a/internal/parser/archive_test.go +++ b/internal/parser/archive_test.go @@ -3,6 +3,8 @@ package parser import ( "archive/tar" "bytes" + "compress/gzip" + "fmt" "os" "path/filepath" "strings" @@ -71,6 +73,80 @@ func TestExtractArchiveFromReaderTXT_TruncatedWhenTooLarge(t *testing.T) { } } +// TestExtractArchiveFromReaderGZ_NoLongerCapsAt50MB is a regression test for +// a real nvidia-bug-report-*.log.gz dump (single gzipped log file, not a tar) +// whose decompressed content was ~53MB and got silently truncated by the old +// fixed 50MB cap, dropping the tail of the log. A single gzipped log with a +// realistic (low) compression ratio should now come through whole. +func TestExtractArchiveFromReaderGZ_NoLongerCapsAt50MB(t *testing.T) { + var plain bytes.Buffer + for i := 0; plain.Len() < 60*1024*1024; i++ { + fmt.Fprintf(&plain, "Aug 24 13:%02d:%02d avi-hgx-b200-ef01 kernel: some log line %d with varying content xyzxyzxyz\n", i%60, (i*7)%60, i) + } + want := plain.Len() + + var gz bytes.Buffer + gw := gzip.NewWriter(&gz) + if _, err := gw.Write(plain.Bytes()); err != nil { + t.Fatalf("write gzip content: %v", err) + } + if err := gw.Close(); err != nil { + t.Fatalf("close gzip writer: %v", err) + } + + files, err := ExtractArchiveFromReader(bytes.NewReader(gz.Bytes()), "nvidia-bug-report-host.log.gz") + if err != nil { + t.Fatalf("extract gzip from reader: %v", err) + } + if len(files) != 1 { + t.Fatalf("expected 1 file, got %d", len(files)) + } + + f := files[0] + if f.Truncated { + t.Fatalf("expected file NOT to be truncated, got message %q", f.TruncatedMessage) + } + if len(f.Content) != want { + t.Fatalf("expected full %d bytes decompressed, got %d", want, len(f.Content)) + } +} + +// TestExtractArchiveFromReaderGZ_AbortsOnDecompressionBomb verifies the +// ratio-based guard that replaced the fixed byte cap still protects against +// a pathological gzip bomb (tiny compressed input, huge decompressed output) +// without requiring the whole bomb to be decompressed first. +func TestExtractArchiveFromReaderGZ_AbortsOnDecompressionBomb(t *testing.T) { + zeros := make([]byte, 50*1024*1024) + + var gz bytes.Buffer + gw := gzip.NewWriter(&gz) + if _, err := gw.Write(zeros); err != nil { + t.Fatalf("write gzip content: %v", err) + } + if err := gw.Close(); err != nil { + t.Fatalf("close gzip writer: %v", err) + } + + files, err := ExtractArchiveFromReader(bytes.NewReader(gz.Bytes()), "bomb.log.gz") + if err != nil { + t.Fatalf("extract gzip from reader: %v", err) + } + if len(files) != 1 { + t.Fatalf("expected 1 file, got %d", len(files)) + } + + f := files[0] + if !f.Truncated { + t.Fatalf("expected bomb to be caught and file marked truncated") + } + if f.TruncatedMessage == "" { + t.Fatalf("expected a truncation message") + } + if len(f.Content) >= len(zeros) { + t.Fatalf("expected bomb guard to abort well before full %d bytes, got %d", len(zeros), len(f.Content)) + } +} + func TestIsSupportedArchiveFilename(t *testing.T) { cases := []struct { name string