Skip to content

Fix out-of-bounds write: keep zend_object last in php_msgpack_base_t - #193

Open
andypost wants to merge 1 commit into
msgpack:masterfrom
andypost:fix-zend-object-layout
Open

andypost wants to merge 1 commit into
msgpack:masterfrom
andypost:fix-zend-object-layout

Conversation

@andypost

@andypost andypost commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

zend_object must be the last member of a custom object struct: its trailing properties_table[1] is sized by zend_object_properties_size(), which is -sizeof(zval) for a class without declared properties, so php_msgpack_base_new() allocates less than sizeof(php_msgpack_base_t).

force_f32 (added in #178) was placed after the embedded zend_object, so it lives outside the allocation. On 32-bit ARM (armhf/armv7), where zval is 8-byte aligned, the store in MessagePack::__construct() lands in the next ZendMM slot of the same bin and clobbers whatever lives there. x86/x86_64 layouts happen to hide it, which is why CI never saw it.

How it shows up

Alpine's armhf/armv7 builders started failing make test with random Termsig=11 after PHP 8.4.27 / 8.5.12 changed allocation patterns enough to expose it. Affected tests are the rand()-driven unpacker ones: 072, 082, 086, 088, 089 — 2–5 of them crash per run:
https://build.alpinelinux.org/buildlogs/build-3-25-armhf/community/php85-pecl-msgpack/php85-pecl-msgpack-3.0.1-r0.log

Debugging under qemu-arm + gdb, a packed array (array(new Obj(1,2,3), new Obj(4,5,6)) in 072, mt_srand(5)) has u.v.flags == 0x14 (PACKED | STATIC_KEYS) on entry to MessagePack::__construct and 0 on entry to pack() — the 1-byte base->force_f32 = 0 store hit it. msgpack_serialize_array() then walks the packed storage as Buckets and dereferences a bogus zend_string.

Verification

On armhf (Alpine edge, PHP 8.5.12): unpatched, every full make test run has 2–5 segfaults; patched, 15/15 full runs are clean (134 passed, 0 failed). Also passes under Alpine's abuild check.

🤖 Generated with Claude Code

@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 68.65%. Comparing base (26c8c70) to head (9cad0b7).
⚠️ Report is 1 commits behind head on master.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #193   +/-   ##
=======================================
  Coverage   68.65%   68.65%           
=======================================
  Files           7        7           
  Lines        1356     1356           
  Branches      248      248           
=======================================
  Hits          931      931           
  Misses        310      310           
  Partials      115      115           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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