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.

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

UAF ext/dom/php_dom.c #17847

Open
YuanchengJiang opened this issue Feb 18, 2025 · 1 comment
Open

UAF ext/dom/php_dom.c #17847

YuanchengJiang opened this issue Feb 18, 2025 · 1 comment

Comments

@YuanchengJiang
Copy link

Description

The following code:

<?php
$doc = new DOMDocument();
$doc->loadXML(<<<XML
<root>
<child/>
</root>
XML);
$xi = $doc->createElementNS('http://www.w3.org/2001/XInclude', 'xi:include');
$xi->setAttribute('href', 'nonexistent');
$fallback = $doc->createElementNS('http://www.w3.org/2001/XInclude', 'xi:fallback');
$xi->appendChild($fallback);
$fallback = $fallback->appendChild($doc->createElement('fallback-child2'));
$xpath = new DOMXPath($doc);
$toReplace = $xpath->query('//child')->item(0);
$toReplace->parentNode->replaceChild($xi, $toReplace);
var_dump(@$doc->xinclude());

Resulted in this output:

=================================================================
==1705699==ERROR: AddressSanitizer: heap-use-after-free on address 0x60c00001e288 at pc 0x000001115682 bp 0x7ffc1e416420 sp 0x7ffc1e416418
READ of size 4 at 0x60c00001e288 thread T0
    #0 0x1115681 in dom_objects_free_storage /home/phpfuzz/WorkSpace/flowfusion/php-src/ext/dom/php_dom.c:1444:13
    #1 0x56282f1 in zend_objects_store_del /home/phpfuzz/WorkSpace/flowfusion/php-src/Zend/zend_objects_API.c:194:4
    #2 0x5739797 in rc_dtor_func /home/phpfuzz/WorkSpace/flowfusion/php-src/Zend/zend_variables.c:57:2
    #3 0x5739a1e in i_zval_ptr_dtor /home/phpfuzz/WorkSpace/flowfusion/php-src/Zend/zend_variables.h:45:4
    #4 0x57397d4 in zval_ptr_dtor /home/phpfuzz/WorkSpace/flowfusion/php-src/Zend/zend_variables.c:84:2
    #5 0x526db21 in _zend_hash_del_el_ex /home/phpfuzz/WorkSpace/flowfusion/php-src/Zend/zend_hash.c:1487:3
    #6 0x526b29d in _zend_hash_del_el /home/phpfuzz/WorkSpace/flowfusion/php-src/Zend/zend_hash.c:1514:2
    #7 0x5284be4 in zend_hash_reverse_apply /home/phpfuzz/WorkSpace/flowfusion/php-src/Zend/zend_hash.c:2230:5
    #8 0x495eecc in shutdown_destructors /home/phpfuzz/WorkSpace/flowfusion/php-src/Zend/zend_execute_API.c:264:4
    #9 0x578249b in zend_call_destructors /home/phpfuzz/WorkSpace/flowfusion/php-src/Zend/zend.c:1335:3
    #10 0x3f77418 in php_request_shutdown /home/phpfuzz/WorkSpace/flowfusion/php-src/main/main.c:1921:3
    #11 0x57ae3ae in do_cli /home/phpfuzz/WorkSpace/flowfusion/php-src/sapi/cli/php_cli.c:1144:3
    #12 0x57a347f in main /home/phpfuzz/WorkSpace/flowfusion/php-src/sapi/cli/php_cli.c:1348:18
    #13 0x7fdf5f301d8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
    #14 0x7fdf5f301e3f in __libc_start_main csu/../csu/libc-start.c:392:3
    #15 0x606174 in _start (/home/phpfuzz/WorkSpace/flowfusion/php-src/sapi/cli/php+0x606174)

0x60c00001e288 is located 8 bytes inside of 120-byte region [0x60c00001e280,0x60c00001e2f8)
freed by thread T0 here:
    #0 0x680dd2 in free (/home/phpfuzz/WorkSpace/flowfusion/php-src/sapi/cli/php+0x680dd2)
    #1 0x7fdf5fced80b in xmlFreeNodeList (/lib/x86_64-linux-gnu/libxml2.so.2+0x6480b)

previously allocated by thread T0 here:
    #0 0x68103d in malloc (/home/phpfuzz/WorkSpace/flowfusion/php-src/sapi/cli/php+0x68103d)
    #1 0x7fdf5fceb6f4 in xmlNewNodeEatName (/lib/x86_64-linux-gnu/libxml2.so.2+0x626f4)

SUMMARY: AddressSanitizer: heap-use-after-free /home/phpfuzz/WorkSpace/flowfusion/php-src/ext/dom/php_dom.c:1444:13 in dom_objects_free_storage
Shadow bytes around the buggy address:
  0x0c187fffbc00: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
  0x0c187fffbc10: fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa fa
  0x0c187fffbc20: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fa
  0x0c187fffbc30: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
  0x0c187fffbc40: fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa fa
=>0x0c187fffbc50: fd[fd]fd fd fd fd fd fd fd fd fd fd fd fd fd fa
  0x0c187fffbc60: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
  0x0c187fffbc70: fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa fa
  0x0c187fffbc80: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c187fffbc90: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
  0x0c187fffbca0: fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
  Shadow gap:              cc
==1705699==ABORTING

To reproduce:

./php-src/sapi/cli/php  ./test.php

Commit:

commit 8af9042405524ae681596b342ffe822af92f4b9f
Author: Arnaud Le Blanc <[email protected]>
Date:   Mon Feb 17 13:46:59 2025 +0100

    Use new ZPP and inline caches in ReflectionProperty::getValue() and variants (#17698)
    
    Apply the following changes to ReflectionProperty::getValue(), ::getRawValue(), ::isInitialized(), ::setValue(), ::setRawValue():
    
    - Pass a cache slot to the property handler
    - Inline the simple case of fetching a declared property
    - Use the new parameter parsing API
    
    This results in run time decrease of 12% to 59% in [micro benchmarks](https://gist.github.com/arnaud-lb/2de08142dcd0c7b49caed21398f44656), when executed with `hyperfine -L version base,opt "/tmp/{version}/sapi/cli/php -d opcache.enable_cli=1 $script"`:
    
    ```
    get-raw-value.php:   -58%
    get-value-dyn.php:   -50%
    get-value-hook.php:  -47%
    get-value.php:       -59%
    is-initialized.php:  -59%
    set-value.php:       -11%
    ```
    
    And a 1.8% decrease in this Doctrine benchmark: https://github.com/arnaud-lb/symfony-demo/compare/505825290d9f1a50d8ce043bb8b451359de21d24...reflection-property-get-value-opt

Configurations:

CC="clang-12" CXX="clang++-12" CFLAGS="-DZEND_VERIFY_TYPE_INFERENCE" CXXFLAGS="-DZEND_VERIFY_TYPE_INFERENCE" ./configure --enable-debug --enable-address-sanitizer --enable-undefined-sanitizer --enable-re2c-cgoto --enable-fpm --enable-litespeed --enable-phpdbg-debug --enable-zts --enable-bcmath --enable-calendar --enable-dba --enable-dl-test --enable-exif --enable-ftp --enable-gd --enable-gd-jis-conv --enable-mbstring --enable-pcntl --enable-shmop --enable-soap --enable-sockets --enable-sysvmsg --enable-zend-test --with-zlib --with-bz2 --with-curl --with-enchant --with-gettext --with-gmp --with-mhash --with-ldap --with-libedit --with-readline --with-snmp --with-sodium --with-xsl --with-zip

Operating System:

Ubuntu 20.04 Host, Docker 0599jiangyc/flowfusion:latest

This report is automatically generated by FlowFusion

Related: #17188, #16777

PHP Version

8af9042

Operating System

No response

@nielsdos
Copy link
Member

nielsdos commented Feb 18, 2025

Looks like a variant of #14702 but with the node reference existing in the xinclude element outside of the fallback.
Ancient bug as well.

@nielsdos nielsdos self-assigned this Feb 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants