diff --git a/FIDELITY.md b/FIDELITY.md index c8ccd2a..23062d0 100644 --- a/FIDELITY.md +++ b/FIDELITY.md @@ -47,6 +47,23 @@ A survey of how other renderers accelerate loading recommended honouring Retry-A - **Test**: `TestFetchModuleSourceHonoursRetryAfter` serves 429 with `Retry-After: 1` for the first second. With the fix it succeeds after about 1.0s. Stash-verified: the old code drops the module at 0.46s. - **Corpus effect**: none measured. The saved Tailwind, Wikipedia and Hacker News renders are pixel-identical before and after, because no module on those pages was rate-limited. The change corrects a case the corpus does not exercise. +## 2026-10-04 (round 161) — security audit: a 62KB image file could allocate 128MB; raster decode now refuses declared canvases above 25 megapixels; SSRF and toolchain findings recorded (engine#261, patch) + +Scope: outbound fetches the engine makes for a page (images, sheets, fonts, modules, fetch/XHR, form posts), the decode path for untrusted bytes, the link hit-map, and the Go toolchain (govulncheck). + +**Found and fixed** +- **Decompression bomb (image decode)**. `codec.Decode` has no size guard, and the PNG decoder allocates the canvas from a valid compressed stream. Measured: a 62KB PNG of zeros declaring 4000×4000 allocates 128MB in 40ms. Fixed: `decodeRaster` reads the declared dimensions with `image.DecodeConfig` and refuses anything above 25 megapixels (about 100MB of RGBA, still admitting a 6000×4000 photograph). Both raster decode sites use it. Test: `TestDecodeRasterRefusesOversizedDeclaredImage`, which fails with the guard disabled. +- **A correction to my own first measurement**: a header alone (IHDR with no pixel data) allocates nothing on the old path. It fails at the missing data. The allocation risk needs a valid compressed stream, as above. The test is named for what it proves (the declared size is refused), not for an allocation it does not reproduce. + +**Checked and found adequate** +- Response bodies are capped on every fetch path (`LimitReader`: 32MB pages, 4MB sheets, 8MB fonts, 16MB images, 48MB module source, 32MB JS responses). +- Scheme restrictions: images, fonts and sheets accept only http(s) and data:. Modules require an http prefix. The link hit-map rejects `javascript:`, `mailto:`, `tel:` and `data:` hrefs. +- JS `fetch`/XHR go through the same client as the engine's own fetches. + +**Recorded, not changed (decisions for the owner)** +- **SSRF is not guarded by the library.** A page can name any URL in an image, sheet, font, module, or script `fetch`, and the engine will fetch it, including private and link-local addresses. `browserproxy` guards this at dial time (`guard.go`, a `net.Dialer.Control` hook). A program that embeds the engine directly must add an equivalent dial guard through `Engine.Client`. The README now says so. +- **Go toolchain**: govulncheck reports six standard-library findings fixed in go1.26.5 and go1.26.6 (`net/http`, `crypto/tls`, `net/url`, `encoding/xml`, `encoding/asn1`). The module pins go1.26.4 in `go.mod` and in CI. Moving the pin is a CI and supply-chain change, so it is left to the owner. + ## 2026-10-04 (round 158) — the settle loop is a small lever: about 0.26s of CPU on tailwindcss.com, all of it full re-cascades, bounded by the network wait (docs only, no behaviour change) Third candidate after the cascade and the image path: the settle-then-render loop. Profiling one Tailwind render shows `settle` at about 12% of CPU samples, and of that the cascade (`CascadeMediaContainers`) is almost all of it, about 0.26s. JavaScript execution is about 0.07s. A render performs five full cascades. Each is a whole-document pass. diff --git a/README.md b/README.md index bf241b7..0d4c796 100644 --- a/README.md +++ b/README.md @@ -196,6 +196,17 @@ while go.dev, tailwindcss.com and the large pkg.go.dev page are slower. Re-run t harness with `cd bench && go run ./cmd/compare -urls urls.txt` (needs a Chrome/Chromium binary). +## Security + +The engine fetches every resource a page names (images, stylesheets, fonts, +scripts and modules, `fetch`/XHR targets, form posts). It does **not** filter +destinations itself, so rendering an untrusted page from a machine with access to +private networks can reach internal addresses. If you embed the engine directly, +add a dial-time guard through `Engine.Client` that refuses loopback, private and +link-local addresses. `browserproxy` ships one (`guard.go`). Decoding is +bounded: raster images declaring more than 25 megapixels are refused, and every +fetch is size-capped. + ## Test ``` diff --git a/images.go b/images.go index ed0c62a..ebe29a8 100644 --- a/images.go +++ b/images.go @@ -4,8 +4,10 @@ package engine import ( + "bytes" "context" "encoding/base64" + "errors" "image" "io" "net/http" @@ -48,6 +50,27 @@ func resizeRaster(src *raster.Image, w, h int, mode resample.Mode) *raster.Image return out } +// maxImagePixels bounds how many pixels one raster image may decode to. A +// file's header can declare a far larger canvas than its compressed body, and +// the decoder allocates that canvas before reading a pixel, so an unchecked +// declared size lets a small file exhaust memory: a 62KB PNG of zeros declaring +// 4000x4000 allocates 128MB in 40ms. Twenty-five megapixels (about 100MB of +// RGBA) still admits a 6000x4000 photograph. +const maxImagePixels = 25_000_000 + +var errImageTooLarge = errors.New("image declares more pixels than the engine decodes") + +// decodeRaster decodes a raster image after checking its declared dimensions. +// Formats the standard image package cannot read a header for fall through to +// the codec unchecked, as before. +func decodeRaster(data []byte) (*raster.Image, error) { + if cfg, _, err := image.DecodeConfig(bytes.NewReader(data)); err == nil && + int64(cfg.Width)*int64(cfg.Height) > maxImagePixels { + return nil, errImageTooLarge + } + return codec.Decode(data) +} + // imgWorkers is the concurrency bound for image fetch+decode: enough to hide // per-image network latency (the dominant cost) without unbounded fan-out. func imgWorkers(n int) int { @@ -295,7 +318,7 @@ func (e *Engine) loadOneImage(ctx context.Context, doc *Document, sm css.StyleMa } return &LoadedImage{Size: [2]float64{float64(w), float64(h)}, Bitmap: b, Data: data, Format: "svg", SourceW: w, SourceH: h} } - src0, err := codec.Decode(data) + src0, err := decodeRaster(data) if err != nil { return nil } @@ -457,7 +480,7 @@ func (e *Engine) loadOneBackground(ctx context.Context, doc *Document, raw strin } return b } - img, err := codec.Decode(data) + img, err := decodeRaster(data) if err != nil { return nil } diff --git a/images_bomb_test.go b/images_bomb_test.go new file mode 100644 index 0000000..6dfa4f9 --- /dev/null +++ b/images_bomb_test.go @@ -0,0 +1,58 @@ +// Copyright (c) the go-webengine/engine authors. +// SPDX-License-Identifier: BSD-3-Clause + +package engine + +import ( + "bytes" + "encoding/binary" + "hash/crc32" + "image" + "image/color" + "image/png" + "testing" +) + +// pngHeaderOnly is a PNG whose IHDR declares w×h but whose body is just the +// signature and that header: a few dozen bytes that claim a huge canvas. +func pngHeaderOnly(w, h uint32) []byte { + var b bytes.Buffer + b.Write([]byte("\x89PNG\r\n\x1a\n")) + ihdr := make([]byte, 13) + binary.BigEndian.PutUint32(ihdr[0:4], w) + binary.BigEndian.PutUint32(ihdr[4:8], h) + ihdr[8], ihdr[9] = 8, 6 // 8-bit RGBA + chunk := append([]byte("IHDR"), ihdr...) + _ = binary.Write(&b, binary.BigEndian, uint32(len(ihdr))) + b.Write(chunk) + _ = binary.Write(&b, binary.BigEndian, crc32.ChecksumIEEE(chunk)) + return b.Bytes() +} + +// A header declaring far more pixels than maxImagePixels is refused before any +// decoding. (A header alone does not allocate on the old path, which fails at +// the missing pixel data; the allocation risk needs a valid compressed stream, +// measured in FIDELITY.md round 161.) +func TestDecodeRasterRefusesOversizedDeclaredImage(t *testing.T) { + _, err := decodeRaster(pngHeaderOnly(50000, 50000)) + if err != errImageTooLarge { + t.Fatalf("decodeRaster(50000x50000 header) err = %v, want errImageTooLarge", err) + } +} + +// Ordinary images still decode. +func TestDecodeRasterDecodesOrdinaryImages(t *testing.T) { + img := image.NewNRGBA(image.Rect(0, 0, 8, 4)) + img.Set(1, 1, color.NRGBA{R: 255, A: 255}) + var buf bytes.Buffer + if err := png.Encode(&buf, img); err != nil { + t.Fatal(err) + } + got, err := decodeRaster(buf.Bytes()) + if err != nil { + t.Fatalf("decodeRaster(8x4 png) error: %v", err) + } + if b := got.Bounds(); b.Dx() != 8 || b.Dy() != 4 { + t.Errorf("decoded bounds = %v, want 8x4", b) + } +}