Reject CBOR break codes outside indefinite-length items

docs/versionhistory.rst

@@ -7,6 +7,10 @@ This library adheres to `Semantic Versioning 2.0 <http://semver.org/>`_.
 
 **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 <https://github.com/agronholm/cbor2/issues/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

rust/decoder.rs

@@ -47,9 +47,10 @@ static FROZEN_DICT: PyImportable = PyImportable::new("builtins", "frozendict");
 enum DecoderResult<'a> {
     BeginFrame(
         Box<DecoderCallback<'a>>,
-        bool,
+        bool, // requested_immutable
         Option<Bound<'a, PyAny>>,
         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<usize>,
     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<PyAny>, dict: &Bound<PyDict>) -> 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<DecoderResult<'py>> = 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);
             }
         }
     }

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(