Repository navigation
reject lengths over UINT32_MAX in msgpack_pack_str/bin/ext/array/map - #1202
Merged
Merged
Conversation
The C packer's header emitters take a size_t length and store it in the 32-bit str32/bin32/ext32/array32/map32 field with a plain cast. On a 64-bit build msgpack_pack_str(pk, 0x100000001) writes db 00 00 00 01 and returns 0, so the body appended next is 4 GiB longer than the header claims and a peer parses the surplus caller bytes as new objects. Refuse a length that does not fit and return -1 before anything is written, the same failure path the write callback uses. The C++ packer already throws container_size_overflow for this input via checked_get_container_size; this is the C counterpart. The check is compiled only when SIZE_MAX > UINT32_MAX, and UINT32_MAX itself is still accepted.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## c_master #1202 +/- ##
============================================
+ Coverage 57.47% 61.53% +4.06%
============================================
Files 10 10
Lines 1124 1144 +20
Branches 129 140 +11
============================================
+ Hits 646 704 +58
+ Misses 478 440 -38 🚀 New features to boost your workflow:
|
Contributor
|
@rajath201, thank you for sending the PR. It looks good to me. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
msgpack_pack_str, msgpack_pack_bin, msgpack_pack_v4raw, msgpack_pack_ext, msgpack_pack_array and msgpack_pack_map take a size_t but store it into a 32-bit header field with a plain cast. On a 64-bit build a 2^32 + 1 byte string gets the header
db 00 00 00 01and the call still returns 0, so the str_body appended next is 4 GiB longer than the header says. A peer decodes one byte as the string and keeps parsing the rest of the caller's bytes as new objects, which hands whoever supplied that blob control over the structure of the stream.Refuse a length that does not fit instead, returning -1 before anything is written, the same failure path the write callback already uses. This is the C counterpart of checked_get_container_size on the C++ side, where the same input throws container_size_overflow. The check only exists when SIZE_MAX > UINT32_MAX so 32-bit builds compile exactly as before, and UINT32_MAX itself still goes out as str32/ext32 (the ext32 maximum from #1184). The new test fails on the current tree and passes with the fix; the full C suite is green under ASan/UBSan with -Werror.