Skip to content

Fix GH-24712: Use-after-free in foreach by reference with destructor - #24714

Open
lazerg wants to merge 2 commits into
php:PHP-8.4from
lazerg:fix/gh-24712-foreach-ref-dtor
Open

lazerg wants to merge 2 commits into
php:PHP-8.4from
lazerg:fix/gh-24712-foreach-ref-dtor

Conversation

@lazerg

@lazerg lazerg commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

FE_FETCH_RW released the old value of the loop variable before binding the reference to it. If that value is an object whose destructor writes to the same variable, the destructor sees the half-released object still in the slot, and the write frees it a second time. The reference is now stored first and the old value released afterwards, the same way ASSIGN_REF does it, and BIND_STATIC and BIND_INIT_STATIC_OR_JMP get the same change.

Fixes #24712

@iliaal

iliaal commented Oct 26, 2026

Copy link
Copy Markdown
Member

ZEND_BIND_STATIC releases the old value before binding the reference in the same way, so this still corrupts the heap with the patch applied:

class C { function __destruct() { $GLOBALS['a'] = null; } }
$a = new C;
static $a;

Worth fixing in the same PR.

@lazerg

lazerg commented Oct 26, 2026

Copy link
Copy Markdown
Contributor Author

Fixed in 0561ddf. BIND_INIT_STATIC_OR_JMP had the same order, so I changed it too and added a test for both.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants