Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

DOMDocument::getElementById() crashes, use after free #2772

Closed
tstarling opened this issue May 27, 2014 · 1 comment
Closed

DOMDocument::getElementById() crashes, use after free #2772

tstarling opened this issue May 27, 2014 · 1 comment
Assignees
@LiraNuna
Labels
crash

Comments

@tstarling
Copy link
Contributor

c_DOMDocument::t_getelementbyid() caches a pointer to the associated c_DOMElement object in the node struct's "_private" member, but fails to increment the reference count. Thus a use-after-free situation can easily be encountered:

$s = '<html><body><div id="x"></div></body></html>';
$dom = new DOMDocument;
$dom->loadHTML($s);
$child = $dom->getElementById('x');
$child = null;
$child = $dom->getElementById('x');

In debug mode, this will assert in assert_refcount_realistic_ns(), with refcount=0x6a6a6a6a. In release mode, it will return the freed object to the userspace and most likely will crash some time later, as in https://bugzilla.wikimedia.org/show_bug.cgi?id=65703

Git blame implicates b568bf9 #2314 @chalet16
@LiraNuna LiraNuna added the crash label Jun 5, 2014
@LiraNuna LiraNuna self-assigned this Jun 6, 2014
@LiraNuna
Copy link
Contributor

LiraNuna commented Jun 6, 2014

Thanks for this report, @tstarling. I have made a fix that uses Array as a cache storage to be GC-aware.

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

@LiraNuna LiraNuna

Labels
crash
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@LiraNuna @tstarling