From c8a1eeb84b9d3dda8f34a301b93bd2f9c976d98e Mon Sep 17 00:00:00 2001 From: xarmian Date: Thu, 15 Oct 2026 18:31:50 -0400 Subject: [PATCH] fix(collab): an awareness frame's entry count is bounded by the bytes that carry it (BUG-3772) (#2204) * fix(collab): an awareness frame's entry count is bounded by the bytes that carry it, before anything is preallocated (BUG-3772) decodeAwarenessFrame preallocated capacity for the sender's count after checking only count <= len(update). Every entry takes at least three bytes, so a 1 MiB frame claiming one entry per byte reserved about 24 MB before the parse failed. The count is now held to (len(update)-j)/3. Claude-Session: https://claude.ai/code/session_01AmLdFp9hPwhW96ZNYbf1Kd * test(collab): the entry-count test says which half pins the bound (BUG-3772) Claude-Session: https://claude.ai/code/session_01AmLdFp9hPwhW96ZNYbf1Kd --- internal/collab/awareness.go | 10 +++++- internal/collab/awareness_test.go | 55 +++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 1 deletion(-) diff --git a/internal/collab/awareness.go b/internal/collab/awareness.go index 50f5bcd5e..18bbfdac1 100644 --- a/internal/collab/awareness.go +++ b/internal/collab/awareness.go @@ -86,6 +86,10 @@ func appendVarUint(b []byte, v uint64) []byte { return append(b, byte(v)) } +// minAwarenessEntryBytes is the smallest encoded entry: a one-byte client ID, +// a one-byte clock and a zero state length. +const minAwarenessEntryBytes = 3 + // decodeAwarenessFrame parses a whole awareness WebSocket frame: the message // type (1), then a length-prefixed update holding a count and, per entry, a // client ID, a clock and a length-prefixed JSON state. @@ -103,7 +107,11 @@ func decodeAwarenessFrame(frame []byte) ([]awarenessEntry, error) { if err != nil { return nil, errAwarenessFrame } - if count > uint64(len(update)) { // every entry is at least 3 bytes + // The count is the sender's word, and the slice below is preallocated + // from it, so it is held to what the bytes can carry: a frame cannot + // make the server reserve room for entries it does not contain (BUG-3772: + // a 1 MiB frame claiming one entry per byte reserved about 24 MB). + if count > uint64(len(update)-j)/minAwarenessEntryBytes { return nil, errAwarenessFrame } out := make([]awarenessEntry, 0, count) diff --git a/internal/collab/awareness_test.go b/internal/collab/awareness_test.go index 7ba51850f..771530654 100644 --- a/internal/collab/awareness_test.go +++ b/internal/collab/awareness_test.go @@ -3,6 +3,7 @@ package collab import ( "bytes" "encoding/hex" + "runtime" "testing" "time" @@ -74,6 +75,60 @@ func TestAwarenessRefusesMalformedFrames(t *testing.T) { } } +// BUG-3772: the entry count is bounded by the bytes that follow it, at three +// bytes an entry, before anything is allocated from it. The accepting half pins +// the bound as no tighter than the bytes allow. The refusing half is an outcome +// check only: the parse loop refuses that frame too, so it passes with the +// bound removed. TestAwarenessLyingCountAllocatesNothingForIt is the +// regression pin. +func TestAwarenessCountBoundedByEntryBytes(t *testing.T) { + // Exactly as many minimal entries as the bytes hold decodes. + const n = 5 + update := appendVarUint(nil, n) + for k := uint64(1); k <= n; k++ { + update = append(update, byte(k), 0x01, 0x00) + } + got, err := decodeAwarenessFrame(awarenessFrame(update)) + if err != nil { + t.Fatalf("%d minimal entries in %d bytes: %v", n, n*minAwarenessEntryBytes, err) + } + if len(got) != n || got[n-1] != (awarenessEntry{clientID: n, clock: 1, live: true}) { + t.Fatalf("decoded %+v", got) + } + // One more than they hold is refused. + update[0] = n + 1 + if _, err := decodeAwarenessFrame(awarenessFrame(update)); err == nil { + t.Fatalf("a count of %d over %d entry bytes decoded", n+1, n*minAwarenessEntryBytes) + } +} + +// BUG-3772: a frame at the WebSocket read limit (handlers_collab.go, +// collabMaxMessageBytes) claiming one entry per byte reserved room for a +// million entries, about 24 MB, before failing to parse them. +func TestAwarenessLyingCountAllocatesNothingForIt(t *testing.T) { + const size = 1 << 20 + update := appendVarUint(nil, size-8) + update = append(update, make([]byte, size-8-len(update))...) + frame := awarenessFrame(update) + + var before, after runtime.MemStats + runtime.ReadMemStats(&before) + _, err := decodeAwarenessFrame(frame) + runtime.ReadMemStats(&after) + if err == nil { + t.Fatal("a count past the frame's entries decoded") + } + if got := after.TotalAlloc - before.TotalAlloc; got > 1<<20 { + t.Fatalf("decoding a %d-byte frame allocated %d bytes", len(frame), got) + } +} + +func awarenessFrame(update []byte) []byte { + frame := appendVarUint(nil, yMessageAwareness) + frame = appendVarUint(frame, uint64(len(update))) + return append(frame, update...) +} + func liveFrame(clientID, clock uint64) []byte { update := appendVarUint(nil, 1) update = appendVarUint(update, clientID)