From 0315e8a723f4937a2bca5691e789a6f1d5631364 Mon Sep 17 00:00:00 2001 From: tonic Date: Wed, 6 May 2026 10:04:48 +0800 Subject: [PATCH] fix(snapshot): bump FetchSnapshotConfig read cap from 1 MiB to 64 MiB MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous cap was set with the comment "config blob is tiny", but the SnapshotConfig embeds a per-file SparseMap for every file in the snapshot. A 4 GiB Windows VM whose guest has many small live allocations (e.g. an Electron app with a Firebase WebSocket signed in) produces sparse maps in the high-hundred-KB range per fragmented file, and the config blob easily exceeds 1 MiB once memory-ranges and overlay.qcow2 are both accounted for. Observed live: 580 KB + 612 KB + base fields = 1.37 MB total config blob. Wake-side symptom was pull hibernation snapshot ...: stream snapshot: fetch snapshot config: parse snapshot config: unexpected end of JSON input — `io.ReadAll(io.LimitReader(body, 1<<20))` truncated the JSON exactly at 1 MiB, leaving an unterminated structure. Bump the cap to 64 MiB (still defends against pathological / malicious manifests) and add a descriptor.Size pre-check so we reject oversize blobs before reading. Add a streaming overflow guard for the case where the descriptor lies about Size. Tests: - TestFetchSnapshotConfigOverOneMiB: builds a real ~1.4 MB SnapshotConfig with two fragmented sparse maps and asserts parse succeeds (would have failed under the old 1 MiB cap). - TestFetchSnapshotConfigRejectsOversizeDescriptor: rejects a descriptor advertising > 64 MiB up front. Co-Authored-By: Claude Opus 4.7 (1M context) --- snapshot/pull.go | 18 ++++++++++- snapshot/snapshot_test.go | 67 +++++++++++++++++++++++++++++++++++++++ store/sync.go | 3 +- 3 files changed, 86 insertions(+), 2 deletions(-) diff --git a/snapshot/pull.go b/snapshot/pull.go index 93b1f73..bcced4f 100644 --- a/snapshot/pull.go +++ b/snapshot/pull.go @@ -146,20 +146,36 @@ func StreamParsed(ctx context.Context, m *manifest.OCIManifest, dl Downloader, o return writeImportTar(ctx, dl, opts.Name, localName, cfg, m.Layers, opts.Writer, opts.Progress) } +// maxSnapshotConfigSize bounds the config blob read to defend against a +// pathological / malicious manifest pointing at an arbitrarily large blob. +// The previous 1 MiB cap was too tight: the config carries per-file sparse +// segment maps (memory-ranges, overlay.qcow2), and a single fragmented file +// can contribute several hundred KB of map JSON on its own. A 4 GiB Windows +// VM with a Firebase-active agent has been observed at ~1.4 MB total config +// (memory-ranges 580 KB + overlay.qcow2 612 KB + base fields). 64 MiB leaves +// headroom for genuinely fragmented snapshots without losing the safety net. +const maxSnapshotConfigSize = 64 << 20 + // FetchSnapshotConfig downloads and parses the snapshot config blob. func FetchSnapshotConfig(ctx context.Context, dl Downloader, name string, desc manifest.Descriptor) (*manifest.SnapshotConfig, error) { if desc.MediaType != manifest.MediaTypeSnapshotConfig { return nil, fmt.Errorf("unexpected config mediaType %q", desc.MediaType) } + if desc.Size > maxSnapshotConfigSize { + return nil, fmt.Errorf("config blob too large: %d > %d", desc.Size, maxSnapshotConfigSize) + } body, err := dl.GetBlob(ctx, name, desc.Digest) if err != nil { return nil, fmt.Errorf("get config blob %s: %w", desc.Digest, err) } defer func() { _ = body.Close() }() - data, err := io.ReadAll(io.LimitReader(body, 1<<20)) // config blob is tiny + data, err := io.ReadAll(io.LimitReader(body, maxSnapshotConfigSize+1)) if err != nil { return nil, fmt.Errorf("read config blob: %w", err) } + if int64(len(data)) > maxSnapshotConfigSize { + return nil, fmt.Errorf("config blob exceeded cap %d while streaming", maxSnapshotConfigSize) + } var cfg manifest.SnapshotConfig if err := json.Unmarshal(data, &cfg); err != nil { return nil, fmt.Errorf("parse snapshot config: %w", err) diff --git a/snapshot/snapshot_test.go b/snapshot/snapshot_test.go index 1c7eb91..cf01566 100644 --- a/snapshot/snapshot_test.go +++ b/snapshot/snapshot_test.go @@ -12,6 +12,7 @@ import ( "testing" "github.com/cocoonstack/epoch/manifest" + "github.com/cocoonstack/epoch/utils" ) // fakeUploader records every blob and manifest write so tests can assert @@ -489,3 +490,69 @@ func TestPushRequiresName(t *testing.T) { t.Fatal("expected error when name is empty") } } + +// Highly fragmented snapshots can produce a config blob with multi-hundred-KB +// per-file sparse maps; total config size of 1.4 MB is realistic. The previous +// 1 MiB LimitReader truncated the JSON and surfaced as +// "parse snapshot config: unexpected end of JSON input" during wake. +func TestFetchSnapshotConfigOverOneMiB(t *testing.T) { + uploader := newFakeUploader() + + // Build a real ~1.4 MB SnapshotConfig — two sparse maps that mirror the + // Windows hibernate case (memory-ranges + overlay.qcow2). + mkMap := func(n int) string { + parts := make([]string, n) + for i := range parts { + parts[i] = `{"o":1234567890,"l":4096}` + } + return "[" + strings.Join(parts, ",") + "]" + } + cfg := manifest.SnapshotConfig{ + SchemaVersion: "v1", + SnapshotID: "01J0", + Hypervisor: "cloud-hypervisor", + Files: map[string]manifest.SnapshotFile{ + "memory-ranges": {Mode: 0o644, SparseMap: mkMap(25_000), SparseSize: 1 << 32}, + "overlay.qcow2": {Mode: 0o644, SparseMap: mkMap(25_000), SparseSize: 1 << 33}, + }, + } + data, err := json.Marshal(cfg) + if err != nil { + t.Fatal(err) + } + if len(data) <= 1<<20 { + t.Fatalf("test setup: config %d bytes is not over the old 1 MiB cap", len(data)) + } + + digest := "sha256:" + utils.SHA256Hex(data) + uploader.blobs[digest] = data + desc := manifest.Descriptor{ + MediaType: manifest.MediaTypeSnapshotConfig, + Digest: digest, + Size: int64(len(data)), + } + + got, err := FetchSnapshotConfig(t.Context(), uploader, "vm-x", desc) + if err != nil { + t.Fatalf("FetchSnapshotConfig: %v", err) + } + if got.SnapshotID != cfg.SnapshotID { + t.Errorf("SnapshotID = %q, want %q", got.SnapshotID, cfg.SnapshotID) + } + if len(got.Files) != 2 { + t.Errorf("Files count = %d, want 2", len(got.Files)) + } +} + +func TestFetchSnapshotConfigRejectsOversizeDescriptor(t *testing.T) { + uploader := newFakeUploader() + desc := manifest.Descriptor{ + MediaType: manifest.MediaTypeSnapshotConfig, + Digest: "sha256:00", + Size: maxSnapshotConfigSize + 1, + } + _, err := FetchSnapshotConfig(t.Context(), uploader, "vm-x", desc) + if err == nil || !strings.Contains(err.Error(), "config blob too large") { + t.Fatalf("err = %v, want 'config blob too large' substring", err) + } +} diff --git a/store/sync.go b/store/sync.go index 1fc46a4..07be91e 100644 --- a/store/sync.go +++ b/store/sync.go @@ -264,7 +264,8 @@ type tagSyncState struct { func (s *Store) getTagSyncState(ctx context.Context, repoName, tagName string) (tagSyncState, error) { var st tagSyncState - err := s.db.QueryRowContext(ctx, + err := s.db.QueryRowContext( + ctx, `SELECT t.digest, t.kind, t.platform_sizes IS NOT NULL FROM tags t JOIN repositories r ON r.id = t.repository_id