Skip to content

Stop CBOR indefinite-length strings from recursing per chunk - #5502

Merged
nlohmann merged 1 commit into
developfrom
cbor-flatten-chunk-recursion
Sep 11, 2026
Merged

nlohmann merged 1 commit into
developfrom
cbor-flatten-chunk-recursion

Conversation

@nlohmann

@nlohmann nlohmann commented Sep 6, 2026 •

Copy link
Copy Markdown
Owner

Closes one vector of #5104.

What

get_cbor_string() and get_cbor_binary() handled the indefinite-length forms (0x7F and 0x5F) by calling themselves once per chunk. Since a chunk may itself be an indefinite-length string, an input of repeated 0x7F bytes reached one native stack frame per input byte: 200,000 of them crash the process with SIGSEGV before a single byte is rejected.

This is the path that blocked #5293, and it is untouched by the container-level work in the rest of this stack.

How

Count the open levels instead of recursing through them. That is enough here because every chunk is appended to the same result — get_bytes() writes at result.size() — so there is no per-level state to keep. The temporary chunk string and its copy into the result go away with the recursion.

The definite-length cases move to get_cbor_string_chunk() / get_cbor_binary_chunk() unchanged, including their error messages, which still name 0x7F and 0x5F because those are handled one level up.

Verification

Compared against develop over the interesting byte sequences — empty, single-chunk, nested, over-closed and truncated forms, both strings and byte arrays, and an indefinite-length map key: identical values, error codes, messages and byte offsets. The 200,000-level input now reports parse_error.110 at byte 200001 instead of crashing.

Relationship to #5325

#5325 proposes rejecting nested indefinite-length chunks per RFC 8949 §3.2.3. This PR is orthogonal and behaviour-preserving: it removes the recursion without changing what is accepted, which keeps the cbor_binary.cbor fixture and its decoding assertions working. If #5325 is later accepted, it becomes a single condition on the level counter here rather than a change to the control flow.

API impact

No breaking changes. Same inputs accepted, same errors, same byte offsets.

Checklist

  • The changes are described in detail, both the what and why.
  • If applicable, an existing issue is referenced.
  • The Code coverage remained at 100%. A test case for every new line of code.
  • If applicable, the documentation is updated.
  • The source code is amalgamated by running make amalgamate.

🤖 Generated with Claude Code

@nlohmann
nlohmann marked this pull request as ready for review September 6, 2026 17:43
@nlohmann nlohmann added the aspect: binary formats BSON, CBOR, MessagePack, UBJSON label Sep 6, 2026
@nlohmann
nlohmann force-pushed the cbor-flatten-chunk-recursion branch from 25a4333 to 00e5d21 Compare September 7, 2026 05:58
Comment thread include/nlohmann/detail/input/binary_reader.hpp Dismissed
Comment thread include/nlohmann/detail/input/binary_reader.hpp Dismissed
Comment thread include/nlohmann/detail/input/binary_reader.hpp Dismissed
Comment thread include/nlohmann/detail/input/binary_reader.hpp Dismissed
Comment thread single_include/nlohmann/json.hpp Dismissed
Comment thread single_include/nlohmann/json.hpp Dismissed
Comment thread single_include/nlohmann/json.hpp Dismissed
Comment thread single_include/nlohmann/json.hpp Dismissed
@nlohmann nlohmann added the review needed It would be great if someone could review the proposed changes. label Sep 7, 2026
@nlohmann
nlohmann force-pushed the cbor-flatten-chunk-recursion branch from 00e5d21 to 271a013 Compare September 8, 2026 11:13
@nlohmann
nlohmann force-pushed the cbor-flatten-chunk-recursion branch from 271a013 to d61ef62 Compare September 9, 2026 08:21
Base automatically changed from binary-readers-move-result to develop September 10, 2026 15:15
@nlohmann
nlohmann force-pushed the cbor-flatten-chunk-recursion branch from d61ef62 to e8c4b9a Compare September 10, 2026 15:15
@nlohmann nlohmann added 🚀 ready to merge Ready to merge - just waiting for CI to complete. and removed review needed It would be great if someone could review the proposed changes. labels Sep 10, 2026
@nlohmann nlohmann added this to the Release 3.13.0 milestone Sep 10, 2026
get_cbor_string() and get_cbor_binary() handled the indefinite-length forms
(0x7F and 0x5F) by calling themselves once per chunk. Each chunk therefore
cost a native stack frame, and since a chunk may itself be an indefinite-
length string, an input of repeated 0x7F bytes reached one frame per input
byte: 200,000 of them crash the process with SIGSEGV before a single byte is
rejected. This is the same defect as #5104, in a path the container-level
work does not touch.

Count the open levels instead of recursing through them. That is enough here
because every chunk is appended to the same result -- get_bytes() writes at
result.size() -- so there is no per-level state to keep. The temporary chunk
string and its copy into the result go away with the recursion.

The definite-length cases move to get_cbor_string_chunk() and
get_cbor_binary_chunk() unchanged, including their error messages, which
still name 0x7F and 0x5F because those are handled one level up.

Behaviour is unchanged. Comparing against develop over the interesting byte
sequences -- empty, single-chunk, nested, over-closed and truncated forms,
both strings and byte arrays, and an indefinite-length map key -- produces
identical values, error codes, messages and byte offsets. The 200,000-level
input now reports parse_error.110 at byte 200001 instead of crashing.

Note that nesting these is not valid CBOR: RFC 8949, Section 3.2.3 forbids
it. This does not change that either way -- it has always been accepted, and
rejecting it is a separate decision (#5317, #5325). Should it be rejected
later, that is now one condition on the level counter rather than a change to
the control flow.

Signed-off-by: Niels Lohmann <mail@nlohmann.me>
@nlohmann
nlohmann force-pushed the cbor-flatten-chunk-recursion branch from e8c4b9a to d31cfac Compare September 11, 2026 06:23
@nlohmann
nlohmann merged commit dd50f0e into develop Sep 11, 2026
63 of 156 checks passed
@nlohmann
nlohmann deleted the cbor-flatten-chunk-recursion branch September 11, 2026 06:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

aspect: binary formats BSON, CBOR, MessagePack, UBJSON L 🚀 ready to merge Ready to merge - just waiting for CI to complete. tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants