Skip to content

check destination capacity in msgpack_vrefbuffer_migrate - #1200

Merged
redboltz merged 1 commit into
msgpack:c_masterfrom
rajath201:vrefbuffer-migrate-grow-check
Oct 30, 2026
Merged

redboltz merged 1 commit into
msgpack:c_masterfrom
rajath201:vrefbuffer-migrate-grow-check

Conversation

@rajath201

Copy link
Copy Markdown

msgpack_vrefbuffer_migrate decides whether to grow to->array by comparing to->tail + nused against vbuf->end, the end of the source's iovec array, which is a separate allocation. Whether to grows therefore depends on heap layout: when its array sits above the source's, the realloc is skipped and the memcpy writes nused iovecs past to->end. Two default-initialized buffers holding 20 refs each are enough for ASan to report a 320-byte heap-buffer-overflow write at vrefbuffer.c:227, the overflow described in #1197.

Compare nused against the free slots in to, the same fix ee24845 made to the C++ vrefbuffer::migrate(), written as a count so the check never forms a pointer past to->end (the < to->end form suggested in the issue would still only grow when there is already room). The realloc now happens exactly when to is short, so the common path does no extra work. The new vrefbuffer_c_migrate_grows_destination test migrates into whichever array sits higher in memory so the old check fails deterministically: on the current tree it overflows under ASan and fails tail <= end without it, including gcc -m32, and with the fix ctest is 6/6 under clang and gcc with and without ASan/UBSan.

Fixes #1197.

msgpack_vrefbuffer_migrate decided whether to grow to->array by
comparing to->tail + nused against vbuf->end, the end of the source's
iovec array. The two arrays are separate allocations, so the result
depended on heap layout rather than on the room left in `to`. When
to->array sat above the source's array the realloc was skipped and the
memcpy wrote nused iovecs past to->end.

Compare nused against the free slots in `to` instead. This is the C
counterpart of the C++ fix in ee24845, written as a count so the check
never forms a pointer past to->end.

Fixes msgpack#1197.
@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

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 61.53%. Comparing base (eabfe29) to head (1c47ce0).
⚠️ Report is 6 commits behind head on c_master.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@             Coverage Diff              @@
##           c_master    #1200      +/-   ##
============================================
+ 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:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@redboltz

Copy link
Copy Markdown
Contributor

@rajath201 , thank you for sending the PR. It looks good to me.

@redboltz
redboltz merged commit ac6bd73 into msgpack:c_master Oct 30, 2026
19 checks passed
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.

3 participants