Skip to content

Commit dcac6e4

Browse files
committed
admin: reject NUL-byte payload smuggling (Codex P2)
decodeCreateTableRequest used dec.More() to enforce a single JSON document per request, but goccy/go-json treats a raw NUL as end-of-input — a body like `{"table_name":...}\x00{"extra":1}` parsed cleanly, dec.More() returned false, and the trailing content was silently dropped. Codex P2 on PR #634 flagged this as a payload-smuggling vector. Fix: read the body once, scan for NUL before decoding. JSON has no need for raw NUL (control characters must be \u-escaped per RFC 8259), so any NUL is a strong signal of either tooling misconfiguration or deliberate smuggling. Reject with 400. The existing dec.More() check stays — it catches the well-formed trailing-token cases (a second `{...}` or trailing `42`) that would otherwise pass NUL-validation but still violate the strict-body contract. Tests: extend TestDynamoHandler_CreateTable_RejectsBadJSON with two NUL vectors — the trailing `{"extra":1}` from the Codex report, and a bare trailing NUL with no extra payload.
1 parent 4b07fce commit dcac6e4

2 files changed

Lines changed: 25 additions & 4 deletions

File tree

internal/admin/dynamo_handler.go

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package admin
22

33
import (
4+
"bytes"
45
"context"
56
"encoding/base64"
67
"errors"
@@ -358,13 +359,27 @@ func decodeCreateTableRequest(body io.Reader) (CreateTableRequest, error) {
358359
if body == nil {
359360
return CreateTableRequest{}, errors.New("request body is empty")
360361
}
361-
dec := json.NewDecoder(body)
362-
dec.DisallowUnknownFields()
363-
var out CreateTableRequest
364-
if err := dec.Decode(&out); err != nil {
362+
raw, err := io.ReadAll(body)
363+
if err != nil {
365364
if IsMaxBytesError(err) {
366365
return CreateTableRequest{}, errors.New("request body exceeds the 64 KiB admin limit")
367366
}
367+
return CreateTableRequest{}, errors.New("request body could not be read")
368+
}
369+
// Reject any NUL byte in the body. JSON has no need for a
370+
// raw NUL (control characters must be \u-escaped), and at
371+
// least one of our decoders (goccy/go-json) treats a raw
372+
// NUL as end-of-input, so a body like
373+
// `{"table_name":...}\x00{"extra":1}` would otherwise sneak
374+
// past dec.More(). Codex P2 on PR #634 flagged this as a
375+
// payload-smuggling vector.
376+
if bytes.IndexByte(raw, 0) >= 0 {
377+
return CreateTableRequest{}, errors.New("request body contains a NUL byte")
378+
}
379+
dec := json.NewDecoder(bytes.NewReader(raw))
380+
dec.DisallowUnknownFields()
381+
var out CreateTableRequest
382+
if err := dec.Decode(&out); err != nil {
368383
return CreateTableRequest{}, errors.New("request body is not valid JSON")
369384
}
370385
// Reject trailing JSON tokens — `{"table_name":"a", ...}{...}`

internal/admin/dynamo_handler_test.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -502,6 +502,12 @@ func TestDynamoHandler_CreateTable_RejectsBadJSON(t *testing.T) {
502502
`{"table_name":"u","partition_key":{"name":"id","type":"S"}} 42`, // trailing scalar
503503
`{"table_name":"foo/bar","partition_key":{"name":"id","type":"S"}}`, // slash in name
504504
`{"table_name":"a/b/c","partition_key":{"name":"id","type":"S"}}`, // multiple slashes
505+
// NUL-delimited smuggling vector (Codex P2). goccy/go-json
506+
// treats raw NUL as end-of-input so dec.More() would
507+
// otherwise return false; the explicit NUL scan catches it.
508+
"{\"table_name\":\"u\",\"partition_key\":{\"name\":\"id\",\"type\":\"S\"}}\x00{\"extra\":1}",
509+
// Lone NUL inside the JSON object — same vector, less obvious.
510+
"{\"table_name\":\"u\",\"partition_key\":{\"name\":\"id\",\"type\":\"S\"}}\x00",
505511
}
506512
for _, body := range cases {
507513
t.Run(body, func(t *testing.T) {

0 commit comments

Comments
 (0)