Skip to content

Guard C++ buffer and zone size arithmetic against wraparound - #1192

Open
vera-rubin wants to merge 1 commit into
msgpack:cpp_masterfrom
vera-rubin:fix/cpp-size-arithmetic-guards
Open

vera-rubin wants to merge 1 commit into
msgpack:cpp_masterfrom
vera-rubin:fix/cpp-size-arithmetic-guards

Conversation

@vera-rubin

Copy link
Copy Markdown

This applies the size checks from #1191 (C buffers and zone) to the C++ headers. A grow request near SIZE_MAX now throws std::bad_alloc instead of letting the size arithmetic wrap around:

  • sbuffer::expand_buffer(): m_size + len (used by write() and read_from()).
  • vrefbuffer::append_ref() and migrate(): the doubled iovec count and the realloc size.
  • zone::allocate_align(): size plus the alignment padding.
  • finalizer_array::push_expand(): the doubled count and the realloc size (cpp11_zone.hpp, cpp03_zone.hpp and the erb template).

The checks use the same std::numeric_limits<...>::max() - x form as the unpacker. Sizes in range behave as before. New tests in test/buffer.cpp and test/zone.cpp check that oversized requests throw and leave the object usable.

If it lands, we may submit it to Google's Patch Rewards program, and we'd be glad to share any reward with the maintainers.

— ChipLab

Apply the size checks that msgpack#1191 added to the C buffers and zone to
their C++ counterparts, so a size near SIZE_MAX is rejected with
std::bad_alloc instead of wrapping to a small value:

- sbuffer::expand_buffer(): m_size + len (used by write() and
  read_from()).
- vrefbuffer::append_ref() and migrate(): the doubled iovec count and
  the realloc size.
- zone::allocate_align(): size + the alignment padding.
- zone finalizer_array::push_expand(): the doubled finalizer count and
  the realloc size (cpp11_zone.hpp, cpp03_zone.hpp and its erb
  template).

The checks use the std::numeric_limits<std::size_t>::max() - x form
already used by the unpacker. Behaviour for in-range sizes is
unchanged.

Add tests for sbuffer and zone.
@codecov-commenter

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 53.84615% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.85%. Comparing base (39cc081) to head (312205d).
⚠️ Report is 26 commits behind head on cpp_master.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@              Coverage Diff               @@
##           cpp_master    #1192      +/-   ##
==============================================
- Coverage       85.36%   79.85%   -5.52%     
==============================================
  Files              80       80              
  Lines            5099     5803     +704     
  Branches          507      634     +127     
==============================================
+ Hits             4353     4634     +281     
- Misses            746     1169     +423     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

This branch has not been deployed

No deployments
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.

2 participants