Skip to content

Commit

Permalink
Fix GH-17201: Dom\TokenList issues with interned string replace
Browse files Browse the repository at this point in the history
If a bucket previously had a non-interned string, and is now replaced
with an interned string, then the type flags still incorrectly state
it's a non-interned string. This leads to the refcount being edited for
interned strings, which in turn can lead to a crash when protect_memory
is set.

Closes GH-17207.
  • Loading branch information
nielsdos committed Dec 17, 2024
1 parent 634c147 commit e247461
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 1 deletion.
2 changes: 2 additions & 0 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ PHP NEWS

- DOM:
. Fixed bug GH-17145 (DOM memory leak). (nielsdos)
. Fixed bug GH-17201 (Dom\TokenList issues with interned string replace).
(nielsdos)

- FFI:
. Fixed bug #79075 (FFI header parser chokes on comments). (nielsdos)
Expand Down
20 changes: 20 additions & 0 deletions ext/dom/tests/gh17201.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
--TEST--
GH-17201 (Dom\TokenList issues with interned string replace)
--EXTENSIONS--
dom
--INI--
opcache.protect_memory=1
--FILE--
<?php
$dom = DOM\XMLDocument::createFromString('<root class="AA B C"/>');
$element = $dom->documentElement;
$list = $element->classList;
$list->replace('AA', 'AB'); // Use interned string
foreach ($list as $entry) {
var_dump($entry);
}
?>
--EXPECT--
string(2) "AB"
string(1) "B"
string(1) "C"
3 changes: 2 additions & 1 deletion ext/dom/token_list.c
Original file line number Diff line number Diff line change
Expand Up @@ -583,7 +583,8 @@ PHP_METHOD(Dom_TokenList, replace)
/* It already exists, remove token instead. */
zend_hash_del_bucket(token_set, bucket);
} else {
Z_STR(bucket->val) = new_token;
/* Need to use ZVAL_STR instead of Z_STR to reset the type flags. */
ZVAL_STR(&bucket->val, new_token);
}

/* 5. Run the update steps. */
Expand Down

0 comments on commit e247461

Please sign in to comment.