From 4537b2d5b942a2c027302518e355790e06e3f64f Mon Sep 17 00:00:00 2001 From: Vincent Gao Date: Tue, 16 Jun 2026 01:41:09 +0200 Subject: [PATCH] Reject CBOR break codes outside indefinite-length items 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 #305. --- docs/versionhistory.rst | 4 ++++ rust/decoder.rs | 51 +++++++++++++++++++++++++++++++++++++---- tests/test_decoder.py | 49 ++++++++++++++++++++++++++++++++++++++- 3 files changed, 99 insertions(+), 5 deletions(-) diff --git a/docs/versionhistory.rst b/docs/versionhistory.rst index 7f2347b9..1ff51113 100644 --- a/docs/versionhistory.rst +++ b/docs/versionhistory.rst @@ -7,6 +7,10 @@ This library adheres to `Semantic Versioning 2.0 `_. **UNRELEASED** +- Fixed the decoder leaking the internal break-marker sentinel (or accepting it as a data item) + when a CBOR break code (``0xff``) appears outside an indefinite-length item, instead of rejecting + such input as ill-formed + (`#305 `_) - Fixed the decoder registering 6-byte strings in the string reference namespace at indices 65536–4294967295 where the encoder does not, desynchronising the namespace and resolving later string references to the wrong value diff --git a/rust/decoder.rs b/rust/decoder.rs index cf90c143..6375586d 100644 --- a/rust/decoder.rs +++ b/rust/decoder.rs @@ -47,9 +47,10 @@ static FROZEN_DICT: PyImportable = PyImportable::new("builtins", "frozendict"); enum DecoderResult<'a> { BeginFrame( Box>, - bool, + bool, // requested_immutable Option>, DisplayName<'a>, + bool, // accepts_break: an indefinite-length container that a break code terminates ), ContinueFrame(bool), CompleteFrame(Bound<'a, PyAny>), @@ -86,6 +87,9 @@ struct StackFrame<'py> { shareable_index: Option, typename: DisplayName<'py>, contains_string_namespace: bool, + // Whether a CBOR break code is a legal terminator here (only indefinite-length + // arrays and maps). A break reaching any other frame is ill-formed input. + accepts_break: bool, } /// Decorates a function to be a two-stage decoder. @@ -565,6 +569,7 @@ impl CBORDecoder { false, None, DisplayName::String("array"), + optional_length.is_none(), )) } else { let mut list = PyList::empty(py); @@ -602,6 +607,7 @@ impl CBORDecoder { false, Some(container), DisplayName::String("array"), + optional_length.is_none(), )) } } @@ -731,7 +737,13 @@ impl CBORDecoder { } }) }; - Ok(BeginFrame(callback, true, None, DisplayName::String("map"))) + Ok(BeginFrame( + callback, + true, + None, + DisplayName::String("map"), + length_or_none.is_none(), + )) } else { fn check_duplicate(key: &Bound, dict: &Bound) -> PyResult<()> { if dict.contains(key)? { @@ -800,6 +812,7 @@ impl CBORDecoder { true, Some(container), DisplayName::String("map"), + length_or_none.is_none(), )) } } @@ -851,6 +864,7 @@ impl CBORDecoder { } else { DisplayName::PythonName(name.clone()) }, + false, )) } else { let callback = @@ -862,6 +876,7 @@ impl CBORDecoder { immutable, None, DisplayName::SemanticTag(tagnum), + false, )) }; } @@ -925,6 +940,7 @@ impl CBORDecoder { true, Some(container), DisplayName::SemanticTag(tagnum), + false, )); } }; @@ -933,6 +949,7 @@ impl CBORDecoder { true, None, DisplayName::String(typename), + false, )) } @@ -1355,6 +1372,7 @@ impl CBORDecoder { true, container, DisplayName::String("set"), + false, )) } } @@ -1591,6 +1609,15 @@ impl CBORDecoder { let result: PyResult> = if let Some(previous_value) = value.take() { // Call the decoder callback of the last frame let frame = frames.last_mut().unwrap(); + // A break code is only a valid terminator inside an indefinite-length + // array or map. Reaching any other frame means it appeared where a data + // item was expected, which is ill-formed (RFC 8949 §3.2.1). + if !frame.accepts_break && previous_value.is(BREAK_MARKER.get(py).unwrap().bind(py)) + { + return Err(CBORDecodeError::new_err( + "CBOR break code (0xff) found outside of an indefinite-length item", + )); + } if let Some(decoder_callback) = frame.decoder_callback.as_mut() { decoder_callback(previous_value, frame.immutable) .map_err(|e| wrap_exception(py, e, &frame.typename)) @@ -1637,7 +1664,13 @@ impl CBORDecoder { }; match result { - Ok(BeginFrame(callback, requested_immutable, container, typename)) => { + Ok(BeginFrame( + callback, + requested_immutable, + container, + typename, + accepts_break, + )) => { if let Some(frame) = frames.last_mut() && let Some(container) = container && let Some(shareable_index) = frame.shareable_index @@ -1655,6 +1688,7 @@ impl CBORDecoder { shareable_index: None, typename, contains_string_namespace: false, + accepts_break, }, )?; } @@ -1688,6 +1722,7 @@ impl CBORDecoder { shareable_index: None, typename: DisplayName::String("string namespace"), contains_string_namespace: true, + accepts_break: false, }, )?; string_namespaces.push(Vec::new()); @@ -1738,6 +1773,7 @@ impl CBORDecoder { shareable_index: Some(shareables.len()), typename: DisplayName::String("shareable value"), contains_string_namespace: false, + accepts_break: false, }, )?; shareables.push(None); @@ -1794,7 +1830,14 @@ impl CBORDecoder { self.available_bytes = 0; self.read_position = 0; } - return Ok(value.expect("stack is empty but final return value is missing")); + let final_value = value.expect("stack is empty but final return value is missing"); + // A naked break code at the top level is ill-formed (RFC 8949 §3.2.1). + if final_value.is(BREAK_MARKER.get(py).unwrap().bind(py)) { + return Err(CBORDecodeError::new_err( + "CBOR break code (0xff) found outside of an indefinite-length item", + )); + } + return Ok(final_value); } } } diff --git a/tests/test_decoder.py b/tests/test_decoder.py index 4b0184de..c8fb2590 100644 --- a/tests/test_decoder.py +++ b/tests/test_decoder.py @@ -396,7 +396,10 @@ def test_string_invalid_utf8(payload: str) -> None: def test_string_oversized() -> None: - with pytest.raises(CBORDecodeEOF, match="premature end of stream"): + # This payload embeds a break code (0xff) inside a definite-length map, which is + # now rejected as ill-formed before the truncated tail is reached (premature end + # of stream is exercised separately). + with pytest.raises(CBORDecodeError, match="break code"): loads(unhexlify("aeaeaeaeaeaeaeaeae0108c29843d90100d8249f0000aeaeffc26ca799")) @@ -1271,6 +1274,50 @@ def test_indefinite_overflow(data: str) -> None: loads(unhexlify(data)) +@pytest.mark.parametrize( + "data", + [ + pytest.param("ff", id="naked"), + pytest.param("ff01", id="before-data"), + pytest.param("8301ff02", id="definite-array-element"), + pytest.param("81ff", id="definite-array-only-element"), + pytest.param("8181ff", id="nested-definite-array"), + pytest.param("a1ff01", id="definite-map-key"), + pytest.param("a101ff", id="definite-map-value"), + pytest.param("d818ff", id="tag-content"), + pytest.param("d81cff", id="shareable"), + pytest.param("d90100ff", id="string-namespace"), + pytest.param("d9d9f7ff", id="self-describe-tag"), + pytest.param("d90102ff", id="set"), + ], +) +def test_break_outside_indefinite(data: str) -> None: + """A break code (0xff) where a data item is expected is ill-formed (RFC 8949 sect. 3.2.1).""" + with pytest.raises(CBORDecodeError, match="break code"): + loads(unhexlify(data)) + + +def test_break_outside_indefinite_disabled() -> None: + # A naked break must be rejected even when indefinite-length items are disabled. + with pytest.raises(CBORDecodeError, match="break code"): + loads(unhexlify("ff"), allow_indefinite=False) + + +@pytest.mark.parametrize( + "data, expected", + [ + pytest.param("9f01ff", [1], id="array"), + pytest.param("9fff", [], id="empty-array"), + pytest.param("bf616101ff", {"a": 1}, id="map"), + pytest.param("9f9f01ffff", [[1]], id="nested-array"), + pytest.param("829f01ff02", [[1], 2], id="indefinite-in-definite"), + ], +) +def test_break_terminates_indefinite(data: str, expected: object) -> None: + # The break code still correctly terminates a real indefinite-length container. + assert loads(unhexlify(data)) == expected + + def test_invalid_cbor() -> None: with pytest.raises(CBORDecodeError): loads(