Address decoder security review: bound collections in max-total

Message

Security review follow-ups (feat/wire-codec):

- F1 (security): max-total now charges collection slot allocation, not only string/bytevector bodies. decode-count charges k x SLOT-COST (dict pairs 2x) toward the running total, so a collection-heavy frame of tiny elements is bounded by max-total instead of only by frame-size x max-count. Regression tests for list/vector/array/dict. - F2 (doc): note that decoding untrusted keyword/symbol names interns them permanently (intern table not GC'd) -> harden before untrusted remote (Enclave) use by gating symbol/keyword decode. Documented in code, README and docs/wire.md; not gated (would break local round-trips). - F3 (doc): note max-depth also bounds host recursion depth; keep the default modest to avoid stack overflow on a hostile deep frame. - F4 (test): malformed-UTF-8 string/keyword/symbol bodies decode with defined behavior (lenient replacement, never a crash/OOB); tests added.

Tests: 121/121 green (native).

Changed

 README.md          | 21 +++++++++++++++++----
 docs/wire.md       | 16 +++++++++++++++-
 src/sigil/wire.sgl | 41 +++++++++++++++++++++++++++++++++--------
 test/test-wire.sgl | 37 +++++++++++++++++++++++++++++++++++++
 4 files changed, 102 insertions(+), 13 deletions(-)

Diff

README.mdmodified

@@ -48,8 +48,8 @@ Enclave). Pass a caps record to bound the work a hostile frame can trigger:

|-------|---------|---------|

| `max-bytes-len:` | 64 MiB | Largest single string / bytevector body. |

| `max-count:` | 16 M | Largest collection (list/vector/array length, dict pairs). |

−51

| `max-depth:` | 256 | Deepest nesting of collections. |

−52

| `max-total:` | 256 MiB | Total body bytes allocated across the whole message. |

+51

| `max-depth:` | 256 | Deepest nesting of collections. Decode is recursive, so this **also bounds host stack depth** — keep it modest; cranking it to tens of thousands reintroduces stack-overflow risk on a hostile deeply-nested frame. |

+52

| `max-total:` | 256 MiB | Total decoded heap: string/bytevector body bytes **plus** an estimated per-element cost for every collection slot, so a collection-heavy frame of tiny elements is bounded too. |

| `max-int-bytes:` | 1024 | Largest integer payload in varint bytes (bounds bignum size). |

## Supported value types

@@ -127,8 +127,21 @@ The decoder never trusts a length it reads:

127

never reads out of bounds; an endless varint is rejected rather than spun on.

128

129

The test suite includes a hostile-input fuzz battery (truncated frames, lying

−130

length prefixes, cap violations, unknown tags, unknown version, varint abuse)

−131

asserting each errors cleanly.

+130

length prefixes, cap violations, unknown tags, unknown version, varint abuse,

+131

invalid UTF-8 bodies) asserting each errors cleanly (or, for malformed UTF-8,

+132

decodes leniently to a string — never a crash or out-of-bounds read).

+133

+134

### Harden before untrusted remote (Enclave) use

+135

+136

One residual risk is **not** bounded by the per-frame caps: decoding a keyword or

+137

symbol *interns* its name permanently (the intern table is not garbage

+138

collected). A hostile remote peer that streams an endless supply of unique

+139

keyword/symbol names can exhaust memory over time. This is safe for local,

+140

in-process bulk payloads (the current use), but before the codec carries

+141

untrusted *remote* traffic over Enclave, symbol/keyword decoding should be gated

+142

— decoded as plain strings unless a schema explicitly opts in. That is a

+143

transport-level (Part 2) decision and is intentionally not enforced here, since

+144

gating it would break local round-trips.

145

146

## Build

147

docs/wire.mdmodified

@@ -155,10 +155,24 @@ An implementation reading untrusted bytes MUST:

155

remaining buffer **before** allocating or copying.

156

3. Enforce configurable limits: maximum single-body length, maximum collection

157

count, maximum nesting depth, maximum total decoded size, maximum integer

−158

magnitude — rejecting **before** allocation.

+158

magnitude — rejecting **before** allocation. The total-size limit should

+159

charge collection allocation (per-element slot cost), not only string/byte

+160

bodies, or a collection-heavy frame escapes the bound.

161

4. Use a bounds-checked cursor so a truncated or lying frame errors cleanly and

162

never reads out of bounds.

163

5. Treat an unknown tag, unknown version, or trailing bytes as hard errors.

+164

6. If decode is recursive, the nesting-depth limit also bounds host stack depth;

+165

keep its default modest so a deeply-nested frame cannot overflow the stack.

+166

7. Decode a malformed UTF-8 string/keyword/symbol body with defined behavior

+167

(lenient replacement, or a clean error) — never a crash or OOB read.

+168

+169

## Hardening for untrusted remote transport

+170

+171

Interning a decoded keyword/symbol name is permanent in some hosts (the intern

+172

table is not collected). A hostile peer streaming endless unique names can

+173

exhaust memory — a risk the per-frame caps do not bound. For untrusted *remote*

+174

traffic, gate symbol/keyword decoding (decode as strings unless a schema opts in)

+175

at the transport layer. For local in-process payloads this does not apply.

176

177

## Versioning

178

src/sigil/wire.sglmodified

@@ -111,6 +111,13 @@

111

;; against the remaining buffer anyway).

112

(define LEN-VARINT-MAX-BYTES 10)

113

+114

;; Estimated bytes a single decoded collection slot costs in the host heap

+115

;; (a cons cell / vector slot + overhead). Charged per element toward

+116

;; `max-total` so a collection-heavy frame of tiny elements is bounded by

+117

;; max-total, not merely by frame-size x max-count. A conservative upper

+118

;; bound; it need not be exact, only proportional.

+119

(define SLOT-COST 16)

+120

121

;; ========== Security caps ==========

122

123

;;; Build a caps record controlling decode limits. Any omitted field takes

@@ -118,8 +125,14 @@

125

;;;

126

;;; - `max-bytes-len` — largest single string/bytevector body (bytes)

127

;;; - `max-count` — largest collection (list/vector length, dict pairs)

−121

;;; - `max-depth` — deepest nesting of collections

−122

;;; - `max-total` — total body bytes allocated across the whole message

+128

;;; - `max-depth` — deepest nesting of collections. NOTE: decode is

+129

;;; recursive, so this ALSO bounds host stack depth —

+130

;;; keep it modest (the default 256 is safe). Cranking

+131

;;; it to tens of thousands reintroduces stack-overflow

+132

;;; risk on a hostile deeply-nested frame.

+133

;;; - `max-total` — total decoded heap charged: string/bytevector body

+134

;;; bytes PLUS an estimated per-element cost for every

+135

;;; collection slot. Bounds total decoded memory.

136

;;; - `max-int-bytes` — largest int payload (zigzag varint bytes), bounds

137

;;; bignum size

138

(define (make-wire-caps (keys: (max-bytes-len 67108864) ; 64 MiB

@@ -487,8 +500,16 @@

500

((= tag TAG-INT)

501

(unzigzag (read-uvarint c (dict-ref caps 'max-int-bytes:))))

502

((= tag TAG-FLOAT) (decode-float c))

+503

;; utf8->string is lenient on malformed UTF-8 (produces replacement

+504

;; chars, never crashes / reads OOB), so a hostile string body is safe.

505

((= tag TAG-STRING) (utf8->string (decode-body c caps)))

506

((= tag TAG-BYTEVECTOR) (decode-body c caps))

+507

;; HARDEN-BEFORE-ENCLAVE: string->keyword/string->symbol INTERN the name

+508

;; permanently (the intern table is not GC'd). A hostile remote peer

+509

;; streaming endless unique names exhausts memory over time — the

+510

;; per-frame caps do not bound it. Mitigation (decode-as-string for

+511

;; untrusted transport unless a schema opts in) is a Part-2 transport

+512

;; decision; not gated here because it would break local round-trips.

513

((= tag TAG-KEYWORD) (string->keyword (utf8->string (decode-body c caps))))

514

((= tag TAG-SYMBOL) (string->symbol (utf8->string (decode-body c caps))))

515

((= tag TAG-CHAR) (decode-char c))

@@ -521,24 +542,27 @@

542

;; integer->char validates the range (rejects > #x10FFFF / surrogates).

543

(integer->char cp)))

544

−524

;; Read a collection count, enforcing depth and count caps.

−525

(define (decode-count c caps depth what)

+545

;; Read a collection count, enforcing depth and count caps, and charge the

+546

;; container's own slot allocation (k * slot-bytes) toward max-total. depth

+547

;; also bounds host recursion depth (decode is recursive) — see max-depth.

+548

(define (decode-count c caps depth what slot-bytes)

549

(when (>= depth (dict-ref caps 'max-depth:))

550

(error "wire-decode: nesting depth exceeds cap"))

551

(let ((k (read-length c what 1)))

552

(when (> k (dict-ref caps 'max-count:))

553

(error "wire-decode: collection count exceeds cap"))

+554

(cur-add-total! c (* k slot-bytes) caps)

555

k))

556

557

(define (decode-list c caps depth)

−534

(let ((k (decode-count c caps depth "list count")))

+558

(let ((k (decode-count c caps depth "list count" SLOT-COST)))

559

(let loop ((i 0) (acc '()))

560

(if (= i k)

561

(reverse acc)

562

(loop (+ i 1) (cons (decode-value c caps (+ depth 1)) acc))))))

563

564

(define (decode-vector c caps depth)

−541

(let* ((k (decode-count c caps depth "vector count"))

+565

(let* ((k (decode-count c caps depth "vector count" SLOT-COST))

566

(v (make-vector k 0)))

567

(let loop ((i 0))

568

(if (= i k)

@@ -548,14 +572,15 @@

572

(loop (+ i 1)))))))

573

574

(define (decode-array c caps depth)

−551

(let ((k (decode-count c caps depth "array count")))

+575

(let ((k (decode-count c caps depth "array count" SLOT-COST)))

576

(let loop ((i 0) (acc '()))

577

(if (= i k)

578

(list->array (reverse acc))

579

(loop (+ i 1) (cons (decode-value c caps (+ depth 1)) acc))))))

580

581

(define (decode-dict c caps depth)

−558

(let ((k (decode-count c caps depth "dict count")))

+582

;; A dict pair holds two references (key + value), so charge 2 slots each.

+583

(let ((k (decode-count c caps depth "dict count" (* 2 SLOT-COST))))

584

;; Each pair is two values, so it needs at least 2 bytes; tighten the

585

;; buffer check accordingly.

586

(when (> (* k 2) (cur-remaining c))

test/test-wire.sglmodified

@@ -238,6 +238,22 @@

238

(wire-decode (bytes->bv (append wire-header (list #x04)

239

(make-continuation-bytes 12)))))))))

240

+241

(test-group "decode - malformed string bodies"

+242

;; F4: a string body carrying invalid UTF-8 must have defined behavior — a

+243

;; string (lenient decode) or a clean error — never a crash or OOB read.

+244

(test "invalid UTF-8 (bad lead bytes)"

+245

(let ((r (guard (e (#t 'clean-error))

+246

(wire-decode (frame #x04 3 #xFF #xFE #x28)))))

+247

(assert-true (or (string? r) (eq? r 'clean-error)))))

+248

(test "invalid UTF-8 (truncated multibyte)"

+249

(let ((r (guard (e (#t 'clean-error))

+250

(wire-decode (frame #x04 2 #xE2 #x82)))))

+251

(assert-true (or (string? r) (eq? r 'clean-error)))))

+252

(test "invalid UTF-8 in a keyword body"

+253

(let ((r (guard (e (#t 'clean-error))

+254

(wire-decode (frame #x06 2 #xC3 #x28)))))

+255

(assert-true (or (keyword? r) (eq? r 'clean-error))))))

+256

257

(test-group "decode - trailing garbage"

258

(test "extra byte after a complete value"

259

(assert-true (raises? (lambda () (wire-decode (frame #x01 #x99))))))

@@ -287,6 +303,27 @@

303

(wire-decode bytes (make-wire-caps max-total: 8)))))))

304

(test "generous total cap decodes"

305

(let ((v (list "aaaa" "bbbb")))

+306

(assert-equal v (wire-decode (wire-encode v) (make-wire-caps max-total: 1000)))))

+307

;; F1 regression: a collection of TINY elements (no bodies) must still be

+308

;; bounded by max-total — the container's own slot allocation is charged.

+309

(test "list of tiny elements charges slots toward max-total"

+310

(let ((bytes (wire-encode (list 1 2 3 4 5 6 7 8 9 10))))

+311

(assert-true (raises? (lambda ()

+312

(wire-decode bytes (make-wire-caps max-total: 8)))))))

+313

(test "vector of tiny elements charges slots toward max-total"

+314

(let ((bytes (wire-encode (vector 1 2 3 4 5 6 7 8 9 10))))

+315

(assert-true (raises? (lambda ()

+316

(wire-decode bytes (make-wire-caps max-total: 8)))))))

+317

(test "array of tiny elements charges slots toward max-total"

+318

(let ((bytes (wire-encode #[1 2 3 4 5 6 7 8 9 10])))

+319

(assert-true (raises? (lambda ()

+320

(wire-decode bytes (make-wire-caps max-total: 8)))))))

+321

(test "dict of tiny pairs charges slots toward max-total"

+322

(let ((bytes (wire-encode #{ a: 1 b: 2 c: 3 d: 4 })))

+323

(assert-true (raises? (lambda ()

+324

(wire-decode bytes (make-wire-caps max-total: 8)))))))

+325

(test "tiny collection within a generous total cap still decodes"

+326

(let ((v (list 1 2 3)))

327

(assert-equal v (wire-decode (wire-encode v) (make-wire-caps max-total: 1000))))))

328

329

(test-group "caps - int magnitude"