Skip to content

Reject CBOR break codes outside indefinite-length items#320

Open
gaoflow wants to merge 1 commit into
agronholm:masterfrom
gaoflow:fix-305-break-marker-leak
Open

Reject CBOR break codes outside indefinite-length items#320
gaoflow wants to merge 1 commit into
agronholm:masterfrom
gaoflow:fix-305-break-marker-leak

Conversation

@gaoflow

@gaoflow gaoflow commented Jun 15, 2026

Copy link
Copy Markdown

Fixes #305.

Problem

A break code (major type 7, subtype 31 / 0xff) is only well-formed as the terminator of an indefinite-length item. When it appears anywhere a data item is expected, the decoder leaks the internal break-marker sentinel to the caller (or accepts it as a real element) instead of rejecting the input:

>>> import cbor2
>>> cbor2.loads(bytes.fromhex('ff'))                          # naked break
<object object at 0x...>                                      # should raise
>>> cbor2.loads(bytes.fromhex('ff'), allow_indefinite=False)
<object object at 0x...>                                      # should raise
>>> cbor2.loads(bytes.fromhex('8301ff02'))                    # break inside a definite array
[1, <object object at 0x...>, 2]                              # should raise

The same leak occurs for definite-length map keys/values (a1ff01, a101ff) and for the content of a tag, shareable value or string-reference namespace (d818ff, d81cff, d90100ff, d9d9f7ff). RFC 8949 sect. 3.2.1 defines such input as not well-formed, and as noted in the issue, "under no circumstances should a naked break marker come out of the decoding process".

Cause

The break marker is produced by decode_special as an ordinary decoded value and flows through the generic frame machinery. Only the indefinite-length array/map callbacks check item.is(break_marker) to terminate; every other consumer (definite-length containers, tags, sets, shareables, string namespaces, and the top-level result) silently passes it through.

Fix

Establish the invariant that a break marker may only be consumed by an indefinite-length container. A new accepts_break flag on StackFrame is set only for indefinite-length array and map frames (indefinite-length strings consume the break internally and never produce a free value). The decode loop then rejects a break marker that reaches any other frame, and one left as the top-level result, with a CBORDecodeError.

Tests

Added test_break_outside_indefinite (12 positions: naked, before-data, definite array element/first/nested, definite map key/value, tag content, shareable, string namespace, self-describe tag, set), test_break_outside_indefinite_disabled, and test_break_terminates_indefinite (5 cases confirming real indefinite arrays/maps, nesting, and indefinite-in-definite still decode). test_string_oversized's fuzz payload embeds a break inside a definite map and now raises the break error before its truncated tail is reached, so its expectation was updated (premature-end-of-stream remains covered by other tests). Full suite passes (462).

I worked on this with AI assistance under my direction and reviewed the change myself.

The decoder returned the internal break-marker sentinel to the caller
whenever a break code (major type 7, subtype 31 / 0xff) appeared where a
data item was expected: at the top level (a naked break), as a
definite-length array element or map key/value, and as the content of a
tag, shareable or string-reference namespace. RFC 8949 sect. 3.2.1
defines such input as not well-formed, so the decoder must reject it.

A break code is only a legal terminator inside an indefinite-length array
or map (indefinite-length strings consume it internally). Mark those two
frame types as accepting a break and reject a break that reaches any
other frame, as well as one left as the top-level result.

Fixes agronholm#305.
@coveralls

Copy link
Copy Markdown

Coverage Status

coverage: 94.952% (+0.06%) from 94.889% — gaoflow:fix-305-break-marker-leak into agronholm:master

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Confusing decoding of standalone break marker; allow_indefinite=False still accepts it

2 participants