diff --git a/internal/server/bug3354_import_bundle_bomb_test.go b/internal/server/bug3354_import_bundle_bomb_test.go new file mode 100644 index 000000000..ccb4ec3c2 --- /dev/null +++ b/internal/server/bug3354_import_bundle_bomb_test.go @@ -0,0 +1,425 @@ +package server + +import ( + "archive/tar" + "bytes" + "compress/gzip" + "errors" + "fmt" + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +// BUG-3354: the bundle cap bounds the COMPRESSED body only. A small gzip +// body could decompress without limit, and entries the import discards were +// inflated in full anyway. The tar stream now has its own ceilings. + +type bundleEntry struct { + name string + body []byte +} + +func gzipTar(t *testing.T, entries []bundleEntry) []byte { + t.Helper() + var buf bytes.Buffer + gzw, _ := gzip.NewWriterLevel(&buf, gzip.BestCompression) + tw := tar.NewWriter(gzw) + for _, e := range entries { + if err := tw.WriteHeader(&tar.Header{Name: e.name, Mode: 0o644, Size: int64(len(e.body))}); err != nil { + t.Fatalf("write header %s: %v", e.name, err) + } + if _, err := tw.Write(e.body); err != nil { + t.Fatalf("write body %s: %v", e.name, err) + } + } + if err := tw.Close(); err != nil { + t.Fatalf("close tar: %v", err) + } + if err := gzw.Close(); err != nil { + t.Fatalf("close gzip: %v", err) + } + return buf.Bytes() +} + +func postBundle(srv *Server, name string, body []byte) *httptest.ResponseRecorder { + req := httptest.NewRequest("POST", "/api/v1/workspaces/import?name="+name, bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/gzip") + req.RemoteAddr = "127.0.0.1:1234" + rr := httptest.NewRecorder() + srv.ServeHTTP(rr, req) + return rr +} + +func exportJSONFrom(t *testing.T) []byte { + t.Helper() + src, slug := testServerWithAttachments(t) + rr := doRequest(src, "GET", "/api/v1/workspaces/"+slug+"/export", nil) + if rr.Code != http.StatusOK { + t.Fatalf("export: %d %s", rr.Code, rr.Body.String()) + } + return rr.Body.Bytes() +} + +func workspaceListed(t *testing.T, srv *Server, name string) bool { + t.Helper() + rr := doRequest(srv, "GET", "/api/v1/workspaces", nil) + if rr.Code != http.StatusOK { + t.Fatalf("list workspaces: %d", rr.Code) + } + return strings.Contains(rr.Body.String(), `"name":"`+name+`"`) +} + +// A body well inside the compressed cap that inflates past the decompressed +// ceiling is refused with 413 before the oversize entry is inflated, and the +// workspace its pad-export.json already minted is rolled back. +func TestBUG3354_DecompressionBombRefused(t *testing.T) { + export := exportJSONFrom(t) + const bodyCap = 1 << 20 // decompressed ceiling: 4 MiB + 200 MiB of metadata room + bomb := gzipTar(t, []bundleEntry{ + {"pad-export.json", export}, + {"filler.bin", make([]byte, 300<<20)}, // 300 MiB of zeros, ~300 KiB compressed + }) + dest, _ := testServerWithAttachments(t) + dest.SetImportBundleMaxBytes(bodyCap) + if int64(len(bomb)) >= bodyCap || int64(300<<20) <= dest.decompressedBundleCap() { + t.Fatalf("precondition: bomb body %d must fit the %d compressed cap and inflate past %d", + len(bomb), bodyCap, dest.decompressedBundleCap()) + } + rr := postBundle(dest, "BombWS", bomb) + if rr.Code != http.StatusRequestEntityTooLarge || !strings.Contains(rr.Body.String(), "bundle_too_large") { + t.Fatalf("bomb: got %d %s, want 413 bundle_too_large", rr.Code, rr.Body.String()) + } + // The refusal tells an operator which setting raises the limit. + if !strings.Contains(rr.Body.String(), "PAD_IMPORT_BUNDLE_MAX_BYTES") || + !strings.Contains(rr.Body.String(), fmt.Sprint(dest.decompressedBundleCap())) { + t.Fatalf("the 413 must state the effective limit and the setting that raises it: %s", rr.Body.String()) + } + if workspaceListed(t, dest, "BombWS") { + t.Fatal("the workspace minted before the bomb entry was not rolled back") + } + + // Control: the same bundle with a filler under the ceiling imports. + ok := gzipTar(t, []bundleEntry{ + {"pad-export.json", export}, + {"filler.bin", make([]byte, 64<<10)}, + }) + ctl, _ := testServerWithAttachments(t) + ctl.SetImportBundleMaxBytes(bodyCap) + if rr := postBundle(ctl, "FitsWS", ok); rr.Code != http.StatusCreated { + t.Fatalf("control bundle under the ceiling: got %d %s", rr.Code, rr.Body.String()) + } +} + +// Many entries, each under the remaining budget, that together pass it are +// refused too: the declared-size check is per entry, so the cumulative +// count is what catches them. +func TestBUG3354_CumulativeEntriesRefused(t *testing.T) { + export := exportJSONFrom(t) + const bodyCap = 1 << 20 // decompressed ceiling: 4 MiB + 200 MiB of metadata room + filler := make([]byte, 50<<20) + entries := []bundleEntry{{"pad-export.json", export}} + for i := 0; i < 5; i++ { + entries = append(entries, bundleEntry{fmt.Sprintf("filler-%d.bin", i), filler}) + } + dest, _ := testServerWithAttachments(t) + dest.SetImportBundleMaxBytes(bodyCap) + rr := postBundle(dest, "ManyWS", gzipTar(t, entries)) + if rr.Code != http.StatusRequestEntityTooLarge || !strings.Contains(rr.Body.String(), "bundle_too_large") { + t.Fatalf("cumulative: got %d %s, want 413 bundle_too_large", rr.Code, rr.Body.String()) + } + if workspaceListed(t, dest, "ManyWS") { + t.Fatal("the partial workspace was not rolled back") + } +} + +// More tar headers than importBundleMaxEntries are refused, however small. +func TestBUG3354_EntryCountRefused(t *testing.T) { + entries := make([]bundleEntry, importBundleMaxEntries+1) + for i := range entries { + entries[i] = bundleEntry{name: fmt.Sprintf("e%d", i)} + } + srv, _ := testServerWithAttachments(t) + rr := postBundle(srv, "CountWS", gzipTar(t, entries)) + if rr.Code != http.StatusRequestEntityTooLarge || !strings.Contains(rr.Body.String(), "entries") { + t.Fatalf("entry count: got %d %s, want 413 naming the entry cap", rr.Code, rr.Body.String()) + } +} + +func TestBUG3354_BudgetReader(t *testing.T) { + read := func(src string, limit int64) (string, error) { + b := &bundleBudgetReader{r: strings.NewReader(src), remaining: limit} + // Bounded rather than io.ReadAll, so a reader that stops making + // progress fails here instead of hanging the test binary. + var got []byte + buf := make([]byte, 3) + for i := 0; i < 100; i++ { + n, err := b.Read(buf) + got = append(got, buf[:n]...) + if err == io.EOF { + return string(got), nil + } + if err != nil { + return string(got), err + } + } + t.Fatalf("reader made no progress over %q (limit %d)", src, limit) + return "", nil + } + // A stream ending exactly at the limit reads to EOF. + if got, err := read("abcd", 4); err != nil || got != "abcd" { + t.Fatalf("exact limit: got %q %v", got, err) + } + // One byte past it fails, having handed over only the budget. + if got, err := read("abcde", 4); !errors.Is(err, errBundleTooLarge) || got != "abcd" { + t.Fatalf("past limit: got %q %v, want errBundleTooLarge after %q", got, err, "abcd") + } + // Under the limit is untouched. + if got, err := read("ab", 4); err != nil || got != "ab" { + t.Fatalf("under limit: got %q %v", got, err) + } +} + +// rawTarEntry writes one ustar entry with an arbitrary typeflag (tar.Writer +// refuses to emit raw extension headers itself), fixing up the checksum. +func rawTarEntry(t *testing.T, name string, flag byte, body []byte) []byte { + t.Helper() + var out bytes.Buffer + tw := tar.NewWriter(&out) + if err := tw.WriteHeader(&tar.Header{Name: name, Mode: 0o644, Size: int64(len(body)), Format: tar.FormatUSTAR}); err != nil { + t.Fatal(err) + } + if _, err := tw.Write(body); err != nil { + t.Fatal(err) + } + if err := tw.Flush(); err != nil { + t.Fatal(err) + } + raw := out.Bytes() + raw[156] = flag + for i := 148; i < 156; i++ { + raw[i] = ' ' + } + sum := 0 + for _, b := range raw[:512] { + sum += int(b) + } + copy(raw[148:156], fmt.Sprintf("%06o\x00 ", sum)) + return raw +} + +func paxRecord(key, value string) string { + payload := key + "=" + value + "\n" + n := len(payload) + 2 + for { + record := fmt.Sprintf("%d %s", n, payload) + if len(record) == n { + return record + } + n = len(record) + } +} + +func gzipBytes(t *testing.T, raw []byte) []byte { + t.Helper() + var out bytes.Buffer + gw := gzip.NewWriter(&out) + if _, err := gw.Write(raw); err != nil { + t.Fatal(err) + } + if err := gw.Close(); err != nil { + t.Fatal(err) + } + return out.Bytes() +} + +// Codex r1 P1: a GNU sparse entry's holes are synthesized by the tar reader +// above the counted stream, so a few KiB of physical bytes declared hundreds +// of MiB. Each entry's declared (logical) size is charged against its own +// budget. +func TestBUG3354_SparseEntriesChargedLogically(t *testing.T) { + raw := rawTarEntry(t, "pad-export.json", tar.TypeReg, exportJSONFrom(t)) + const logicalSize = 96 << 20 + for i := 0; i < 3; i++ { + pax := paxRecord("GNU.sparse.major", "0") + + paxRecord("GNU.sparse.minor", "1") + + paxRecord("GNU.sparse.size", fmt.Sprint(logicalSize)) + + paxRecord("GNU.sparse.numblocks", "1") + + paxRecord("GNU.sparse.map", "0,0") + raw = append(raw, rawTarEntry(t, "pax", tar.TypeXHeader, []byte(pax))...) + raw = append(raw, rawTarEntry(t, fmt.Sprintf("filler-%d.bin", i), tar.TypeReg, nil)...) + } + raw = append(raw, make([]byte, 1024)...) + srv, _ := testServerWithAttachments(t) + srv.SetImportBundleMaxBytes(1 << 20) + if 3*logicalSize <= srv.decompressedBundleCap() { + t.Fatalf("precondition: %d logical bytes must pass the %d ceiling", 3*logicalSize, srv.decompressedBundleCap()) + } + rr := postBundle(srv, "SparseWS", gzipBytes(t, raw)) + if rr.Code != http.StatusRequestEntityTooLarge || !strings.Contains(rr.Body.String(), "bundle_too_large") { + t.Fatalf("sparse entries: got %d %s, want 413 bundle_too_large", rr.Code, rr.Body.String()) + } + if workspaceListed(t, srv, "SparseWS") { + t.Fatal("the partial workspace was not rolled back") + } +} + +// Codex r1 P2: extension headers are consumed inside Next, invisible to the +// entry count. The header blocks Next reads are counted instead. +func TestBUG3354_ExtensionHeaderChainRefused(t *testing.T) { + raw := rawTarEntry(t, "pad-export.json", tar.TypeReg, exportJSONFrom(t)) + extension := rawTarEntry(t, "pax", tar.TypeXHeader, nil) // one block each + raw = append(raw, bytes.Repeat(extension, int(importBundleMaxHeaderBlocks)+1)...) + raw = append(raw, rawTarEntry(t, "filler.bin", tar.TypeReg, nil)...) + raw = append(raw, make([]byte, 1024)...) + srv, _ := testServerWithAttachments(t) + srv.SetImportBundleMaxBytes(1 << 20) + body := gzipBytes(t, raw) + if int64(len(body)) >= srv.effectiveImportBundleMaxBytes() || int64(len(raw)) >= srv.decompressedBundleCap() { + t.Fatalf("precondition: the chain must fit both byte ceilings (body %d, raw %d)", len(body), len(raw)) + } + rr := postBundle(srv, "ChainWS", body) + if rr.Code != http.StatusRequestEntityTooLarge || !strings.Contains(rr.Body.String(), "header blocks") { + t.Fatalf("extension-header chain: got %d %s, want 413 naming the header-block cap", rr.Code, rr.Body.String()) + } + if workspaceListed(t, srv, "ChainWS") { + t.Fatal("the partial workspace was not rolled back") + } +} + +// The counted reader itself trips, not the declared-size check: the entry +// declares a size that fits, and only its padding crosses the ceiling. The +// deferred conversion answers 413 and the minted workspace is rolled back. +func TestBUG3354_ReaderOverflowConvertsTo413(t *testing.T) { + srv, _ := testServerWithAttachments(t) + srv.SetImportBundleMaxBytes(1 << 20) + var out bytes.Buffer + out.Write(rawTarEntry(t, "pad-export.json", tar.TypeReg, exportJSONFrom(t))) + tw := tar.NewWriter(&out) + size := srv.decompressedBundleCap() - int64(out.Len()) - 512 - 1 + if err := tw.WriteHeader(&tar.Header{Name: "filler.bin", Mode: 0o644, Size: size}); err != nil { + t.Fatal(err) + } + chunk := make([]byte, 1<<20) + for left := size; left > 0; { + n := int64(len(chunk)) + if n > left { + n = left + } + if _, err := tw.Write(chunk[:n]); err != nil { + t.Fatal(err) + } + left -= n + } + if err := tw.Close(); err != nil { + t.Fatal(err) + } + rr := postBundle(srv, "PaddingWS", gzipBytes(t, out.Bytes())) + if rr.Code != http.StatusRequestEntityTooLarge || !strings.Contains(rr.Body.String(), "bundle_too_large") { + t.Fatalf("reader overflow: got %d %s, want 413 bundle_too_large", rr.Code, rr.Body.String()) + } + if workspaceListed(t, srv, "PaddingWS") { + t.Fatal("the partial workspace was not rolled back") + } +} + +// Codex r2: the tar reader accepts a negative base-256 size on a directory +// header, and charging it raised the logical budget, re-opening the sparse +// bypass. A negative size is refused outright. +func TestBUG3354_NegativeSizeRefused(t *testing.T) { + raw := rawTarEntry(t, "pad-export.json", tar.TypeReg, exportJSONFrom(t)) + dir := rawTarEntry(t, "dir/", tar.TypeDir, nil) + // Size field (bytes 124..135) in base-256: high bit set, two's complement. + negSize := int64(-1) << 40 + neg := uint64(negSize) + for i := 124; i < 128; i++ { + dir[i] = 0xff + } + for i := 0; i < 8; i++ { + dir[128+i] = byte(neg >> (56 - 8*i)) + } + for i := 148; i < 156; i++ { + dir[i] = ' ' + } + sum := 0 + for _, b := range dir[:512] { + sum += int(b) + } + copy(dir[148:156], fmt.Sprintf("%06o\x00 ", sum)) + raw = append(raw, dir...) + // Behind it, sparse entries whose logical size only fits the budget the + // negative size would have inflated. + for i := 0; i < 3; i++ { + pax := paxRecord("GNU.sparse.major", "0") + + paxRecord("GNU.sparse.minor", "1") + + paxRecord("GNU.sparse.size", fmt.Sprint(96<<20)) + + paxRecord("GNU.sparse.numblocks", "1") + + paxRecord("GNU.sparse.map", "0,0") + raw = append(raw, rawTarEntry(t, "pax", tar.TypeXHeader, []byte(pax))...) + raw = append(raw, rawTarEntry(t, fmt.Sprintf("filler-%d.bin", i), tar.TypeReg, nil)...) + } + raw = append(raw, make([]byte, 1024)...) + + // Precondition: the reader really does hand back the negative size. + tr := tar.NewReader(bytes.NewReader(raw)) + if _, err := tr.Next(); err != nil { + t.Fatalf("read export: %v", err) + } + if hdr, err := tr.Next(); err != nil || hdr.Size >= 0 { + t.Fatalf("precondition: want a negative-size directory header, got %+v, %v", hdr, err) + } + + srv, _ := testServerWithAttachments(t) + srv.SetImportBundleMaxBytes(1 << 20) + rr := postBundle(srv, "NegWS", gzipBytes(t, raw)) + if rr.Code != http.StatusBadRequest || !strings.Contains(rr.Body.String(), "negative size") { + t.Fatalf("negative size: got %d %s, want 400 naming the negative size", rr.Code, rr.Body.String()) + } + if workspaceListed(t, srv, "NegWS") { + t.Fatal("the partial workspace was not rolled back") + } +} + +// countingBody counts the bytes the handler pulls from the request body. +type countingBody struct { + r io.Reader + n int64 +} + +func (c *countingBody) Read(p []byte) (int, error) { + n, err := c.r.Read(p) + c.n += int64(n) + return n, err +} + +// Codex r3: Next walks a whole extension chain internally, so a header limit +// checked after it returns is refused only after all the work is done. The +// limit is enforced inside the walk: a chain of three times the cap, under +// the default byte ceilings, is refused having read about a third of it. +func TestBUG3354_HeaderLimitStopsTheWalk(t *testing.T) { + raw := rawTarEntry(t, "pad-export.json", tar.TypeReg, exportJSONFrom(t)) + extension := rawTarEntry(t, "pax", tar.TypeXHeader, nil) // one block each + raw = append(raw, bytes.Repeat(extension, 3*int(importBundleMaxHeaderBlocks))...) + raw = append(raw, rawTarEntry(t, "filler.bin", tar.TypeReg, nil)...) + raw = append(raw, make([]byte, 1024)...) + body := gzipBytes(t, raw) + + srv, _ := testServerWithAttachments(t) + if int64(len(raw)) >= srv.decompressedBundleCap() { + t.Fatalf("precondition: the chain must fit the byte ceiling, so only the header limit can stop it") + } + cb := &countingBody{r: bytes.NewReader(body)} + req := httptest.NewRequest("POST", "/api/v1/workspaces/import?name=WalkWS", cb) + req.Header.Set("Content-Type", "application/gzip") + req.RemoteAddr = "127.0.0.1:1234" + rr := httptest.NewRecorder() + srv.ServeHTTP(rr, req) + if rr.Code != http.StatusRequestEntityTooLarge || !strings.Contains(rr.Body.String(), "header blocks") { + t.Fatalf("chain: got %d %s, want 413 naming the header-block cap", rr.Code, rr.Body.String()) + } + if cb.n*2 > int64(len(body)) { + t.Fatalf("refused only after reading %d of %d body bytes: the limit did not stop the walk", cb.n, len(body)) + } +} diff --git a/internal/server/handlers_import_bundle.go b/internal/server/handlers_import_bundle.go index 5205a921b..8c039e5ef 100644 --- a/internal/server/handlers_import_bundle.go +++ b/internal/server/handlers_import_bundle.go @@ -44,6 +44,128 @@ const defaultImportBundleMaxBytes int64 = 2 << 30 // 2 GiB // item count, not attachment sizes. const importMetadataMaxBytes int64 = 100 << 20 // 100 MiB +// BUG-3354: the body cap above bounds the COMPRESSED bytes. Gzip expands up +// to ~1000:1, so a 2 GiB body could decompress to terabytes, and entries the +// import discards (unmanifested blobs, unknown names) were inflated in full +// just to be thrown away: hours of CPU for one authenticated request. Two +// ceilings now apply to the tar stream itself, both answered with 413. +// +// importBundleExpansionFactor bounds the DECOMPRESSED bytes at a multiple of +// the body cap, plus room for the two metadata files (see +// decompressedBundleCap): 8 GiB + 200 MiB at the default 2 GiB. It is an +// absolute ceiling, not a ratio of the body actually sent, so a 200 MiB +// bundle may still expand 40x. Attachments are mostly images, which barely +// compress; the metadata allowance is separate because JSON compresses far +// more than 4x and its cap does not move with the body cap. +const importBundleExpansionFactor int64 = 4 + +// importBundleMaxEntries bounds the number of tar headers read. Receipt +// (2026-10-02): the largest workspace on the dev instance holds 558 +// attachment rows (8 workspaces, mean 128), and an export writes one entry per +// attachment plus two metadata files, so this is ~180x the largest observed. +const importBundleMaxEntries = 100_000 + +// importBundleMaxHeaderBlocks bounds the 512-byte header blocks read, +// extension headers included (PAX records, GNU long names), which the entry +// count above cannot see. An exported entry costs one header block, or a few +// more with a long name; real exports have hundreds of entries (receipt +// above), so two blocks per permitted entry still leaves wide headroom. +const importBundleMaxHeaderBlocks int64 = 2 * importBundleMaxEntries + +// errBundleTooLarge is returned by bundleBudgetReader once the decompressed +// stream passes its ceiling. +var errBundleTooLarge = errors.New("bundle exceeds its decompressed size limit") + +// bundleBudgetReader counts the bytes read through it and fails once more +// than limit bytes have been read. A stream that ends EXACTLY at the limit +// still reads to EOF. +type bundleBudgetReader struct { + r io.Reader + remaining int64 + exceeded bool + + // headerWindow, while headerArmed, is a second, temporary limit around + // each tar.Reader.Next call: the header bytes still allowed. Next walks + // a whole PAX/GNU extension chain internally, so a limit checked after + // it returns bounds nothing (codex r3); this one stops the walk. + headerArmed bool + headerWindow int64 + headersTripped bool +} + +// errBundleTooManyHeaders is returned once a Next call reads past its +// header window. +var errBundleTooManyHeaders = errors.New("bundle exceeds its tar header limit") + +func (b *bundleBudgetReader) Read(p []byte) (int, error) { + if b.headersTripped { + return 0, errBundleTooManyHeaders + } + if !b.headerArmed { + return b.read(p) + } + if b.headerWindow <= 0 { + b.headersTripped = true + return 0, errBundleTooManyHeaders + } + if int64(len(p)) > b.headerWindow { + p = p[:b.headerWindow] + } + n, err := b.read(p) + b.headerWindow -= int64(n) + return n, err +} + +func (b *bundleBudgetReader) read(p []byte) (int, error) { + if b.exceeded { + return 0, errBundleTooLarge + } + if b.remaining <= 0 { + // At the limit: one probe byte tells a stream that ends here from + // one that keeps going. + var probe [1]byte + n, err := b.r.Read(probe[:]) + if n > 0 { + b.exceeded = true + return 0, errBundleTooLarge + } + return 0, err + } + if int64(len(p)) > b.remaining { + p = p[:b.remaining] + } + n, err := b.r.Read(p) + b.remaining -= int64(n) + return n, err +} + +// decompressedBundleCap is the ceiling on the tar stream: a multiple of the +// body cap for attachment bytes, plus both metadata files at their own cap. +func (s *Server) decompressedBundleCap() int64 { + return s.effectiveImportBundleMaxBytes()*importBundleExpansionFactor + 2*importMetadataMaxBytes +} + +// decompressedCapMessage is the 413 for the decompressed ceiling. It states +// the effective limit and how it is derived, so an operator with a +// legitimately large export knows which setting raises it. +func (s *Server) decompressedCapMessage(detail string) string { + return fmt.Sprintf("Bundle expands past %d bytes once decompressed%s. The limit is %dx the import body cap (%d bytes) plus %d bytes for metadata; raise PAD_IMPORT_BUNDLE_MAX_BYTES on this server to import a larger export.", + s.decompressedBundleCap(), detail, importBundleExpansionFactor, s.effectiveImportBundleMaxBytes(), 2*importMetadataMaxBytes) +} + +// effectiveImportBundleMaxBytes is the compressed-body cap in force. +func (s *Server) effectiveImportBundleMaxBytes() int64 { + if s.importBundleMaxBytes > 0 { + return s.importBundleMaxBytes + } + return defaultImportBundleMaxBytes +} + +// bundleTooLargeError is the 413 every bundle ceiling answers with. +func bundleTooLargeError(message string) *importStatusError { + return &importStatusError{status: http.StatusRequestEntityTooLarge, code: "bundle_too_large", message: message} +} + // effectiveBlobMaxBytes returns the per-blob ceiling for bundle // import — matches whatever the upload handler accepts so an // operator who raised PAD_ATTACHMENT_MAX_BYTES on the source can @@ -111,10 +233,7 @@ func (s *Server) handleImportWorkspaceBundle(w http.ResponseWriter, r *http.Requ return } // Bound the request body BEFORE the gzip reader spools any of it. - maxBytes := s.importBundleMaxBytes - if maxBytes <= 0 { - maxBytes = defaultImportBundleMaxBytes - } + maxBytes := s.effectiveImportBundleMaxBytes() r.Body = http.MaxBytesReader(w, r.Body, maxBytes) gz, err := gzip.NewReader(r.Body) @@ -284,8 +403,21 @@ func (s *Server) importBundle(req *http.Request, r io.Reader, newName string, mi // context the JSON path does rather than re-deriving owner and source // from the request down here (BUG-2809). ownerID := mint.OwnerID - tr := tar.NewReader(r) + // BUG-3354: every byte the tar reader sees, header and padding included, + // is counted against the decompressed ceiling. + budget := &bundleBudgetReader{r: r, remaining: s.decompressedBundleCap()} + decompressedCap := budget.remaining + tr := tar.NewReader(budget) blobCap := s.effectiveBlobMaxBytes() + entries := 0 + // Two more counters, because the physical byte count alone is not the + // whole cost (codex r1). A sparse entry's holes are synthesized by the tar + // reader ABOVE the counted stream, so logical bytes get their own budget, + // charged by each entry's declared size. And Next consumes PAX and GNU + // extension headers internally, so the entry count cannot see them; the + // header blocks Next reads are counted instead. + logicalRemaining := decompressedCap + headerBlocks := int64(0) var ws *models.Workspace @@ -319,6 +451,14 @@ func (s *Server) importBundle(req *http.Request, r io.Reader, newName string, mi } panic(p) } + // Whichever read noticed it (a header, a blob, a skipped entry), an + // exhausted budget is the bundle's size, not a malformed stream. + if retErr != nil && budget.exceeded { + retErr = bundleTooLargeError(s.decompressedCapMessage("")) + } + if retErr != nil && budget.headersTripped { + retErr = bundleTooLargeError(fmt.Sprintf("Bundle has too many entries (more than %d tar header blocks)", importBundleMaxHeaderBlocks)) + } if retErr != nil && result == nil && ws != nil { result = ws } @@ -330,14 +470,50 @@ func (s *Server) importBundle(req *http.Request, r io.Reader, newName string, mi manifestSeen := false for { + before := budget.remaining + // Arm the header window for this Next: the blocks still allowed, + // plus one for the previous entry's padding. + budget.headerArmed, budget.headerWindow = true, (importBundleMaxHeaderBlocks-headerBlocks+1)*512 hdr, err := tr.Next() + budget.headerArmed = false + // Every entry's body is read to its end before the next Next (the + // skip arms below drain it), so what Next consumed is headers plus at + // most one block of the previous entry's padding. + headerBlocks += (before - budget.remaining) / 512 + if budget.headersTripped || headerBlocks > importBundleMaxHeaderBlocks { + return ws, bundleTooLargeError(fmt.Sprintf("Bundle has too many entries (more than %d tar header blocks)", importBundleMaxHeaderBlocks)) + } if err == io.EOF { break } if err != nil { return nil, fmt.Errorf("read tar entry: %w", err) } + entries++ + if entries > importBundleMaxEntries { + return ws, bundleTooLargeError(fmt.Sprintf("Bundle has more than %d entries", importBundleMaxEntries)) + } + // The tar reader accepts a NEGATIVE size on some entry types (base-256 + // encoding), and charging one would raise the budget it is charged to. + // No exporter writes one. + if hdr.Size < 0 { + return ws, &importStatusError{ + status: http.StatusBadRequest, code: "bad_bundle", + message: fmt.Sprintf("Bundle entry %q declares a negative size", hdr.Name), + } + } + // Refuse an entry that declares more than the budget has left before + // inflating any of it. hdr.Size is the LOGICAL size, holes included. + if hdr.Size > logicalRemaining { + return ws, bundleTooLargeError(s.decompressedCapMessage(fmt.Sprintf(" (entry %q declares %d bytes)", hdr.Name, hdr.Size))) + } + logicalRemaining -= hdr.Size if hdr.Typeflag != tar.TypeReg && hdr.Typeflag != tar.TypeRegA { //nolint:staticcheck // TypeRegA accepted for older bundles + // Drain it here rather than leave it to Next, so Next's + // consumption above stays headers only. + if _, err := io.Copy(io.Discard, io.LimitReader(tr, hdr.Size)); err != nil { + return ws, fmt.Errorf("skip entry %s: %w", hdr.Name, err) + } continue }