Bug #81068 [PATCH]: Double free in realpath_cache_clean()

Edit report at https://bugs.php.net/bug.php?id=81068&edit=1 ID: 81068 Patch added by: dimitry@andric.com Reported by: dimitry at andric dot com Summary: Double free in realpath_cache_clean() Status: Open Type: Bug Package: Apache2 related Operating System: Windows PHP Version: master-Git-2021-05-21 (Git) Block user comment: N Private report: N New Comment: The following patch has been added/updated: Patch Name: fix-realpath-cache-clean-segfault-1 Revision: 1621616117 URL: https://bugs.php.net/patch-display.php?bug=81068&patch=fix-realpath-cache-clean-segfault-1&revision=1621616117 Previous Comments: ------------------------------------------------------------------------ [2021-05-21 16:53:20] dimitry at andric dot com Description: ------------ Using php-src master 77ce2537f7 (ZTS enabled), with Apache 2.4.46 x64 from Apache Lounge, on Windows 10 [Version 10.0.19042.985]. I noticed an access violation upon stopping the Apache service. As this is rather hard to debug, I built php with address sanitizer, and used "httpd.exe -X" to run it within the Visual Studio debugger. If you let it start up, then do at least one HTTP request (for example, just http://localhost/), and then control-C the httpd.exe process, it will attempt to shutdown, and it will effectively call the realpath_cache_clean() function more than once. This is because TSRM calls cwd_globals_ctor() more than once, so the destructor, cwd_globals_dtor() is also called more than once. The second time through realpath_cache_clean(), ASan gives: ================================================================= ==19272==ERROR: AddressSanitizer: heap-use-after-free on address 0x12d32c603e08 at pc 0x7ffacc5493a5 bp 0x0098272cf5d0 sp 0x0098272cf5d8 READ of size 8 at 0x12d32c603e08 thread T0 #0 0x7ffacc5493a4 in realpath_cache_clean C:\Users\Dim\Source\php-src\Zend\zend_virtual_cwd.c:346 #1 0x7ffacc54aa1d in cwd_globals_dtor C:\Users\Dim\Source\php-src\Zend\zend_virtual_cwd.c:155 #2 0x7ffacc745b7f in tsrm_shutdown C:\Users\Dim\Source\php-src\TSRM\TSRM.c:192 #3 0x7ffb0350ce21 in php_apache_server_shutdown C:\Users\Dim\Source\php-src\sapi\apache2handler\sapi_apache2.c:419 #4 0x61bde68d in apr_pool_destroy+0x6d (C:\Apache24\bin\libapr-1.dll+0x6eece68d) #5 0x61bde66e in apr_pool_destroy+0x4e (C:\Apache24\bin\libapr-1.dll+0x6eece66e) #6 0x7ff7c211127f in OPENSSL_Applink+0x27f (C:\Apache24\bin\httpd.exe+0x14000127f) #7 0x7ff7c2111eeb in OPENSSL_Applink+0xeeb (C:\Apache24\bin\httpd.exe+0x140001eeb) #8 0x7ff7c2112fa3 in OPENSSL_Applink+0x1fa3 (C:\Apache24\bin\httpd.exe+0x140002fa3) #9 0x7ffb1cd57033 in BaseThreadInitThunk+0x13 (C:\Windows\System32\KERNEL32.DLL+0x180017033) #10 0x7ffb1eaa2650 in RtlUserThreadStart+0x20 (C:\Windows\SYSTEM32\ntdll.dll+0x180052650) 0x12d32c603e08 is located 1288 bytes inside of 12320-byte region [0x12d32c603900,0x12d32c606920) freed by thread T0 here: #0 0x7fface90c2c2 in _asan_wrap_GlobalSize+0x49607 (C:\Users\Dim\Source\php-src\x64\Debug_TS\clang_rt.asan_dbg_dynamic-x86_64.dll+0x18004c2c2) #1 0x7ffacc745c42 in tsrm_shutdown C:\Users\Dim\Source\php-src\TSRM\TSRM.c:200 #2 0x7ffb0350ce21 in php_apache_server_shutdown C:\Users\Dim\Source\php-src\sapi\apache2handler\sapi_apache2.c:419 #3 0x61bde68d in apr_pool_destroy+0x6d (C:\Apache24\bin\libapr-1.dll+0x6eece68d) #4 0x61bde66e in apr_pool_destroy+0x4e (C:\Apache24\bin\libapr-1.dll+0x6eece66e) #5 0x7ff7c211127f in OPENSSL_Applink+0x27f (C:\Apache24\bin\httpd.exe+0x14000127f) #6 0x7ff7c2111eeb in OPENSSL_Applink+0xeeb (C:\Apache24\bin\httpd.exe+0x140001eeb) #7 0x7ff7c2112fa3 in OPENSSL_Applink+0x1fa3 (C:\Apache24\bin\httpd.exe+0x140002fa3) #8 0x7ffb1cd57033 in BaseThreadInitThunk+0x13 (C:\Windows\System32\KERNEL32.DLL+0x180017033) #9 0x7ffb1eaa2650 in RtlUserThreadStart+0x20 (C:\Windows\SYSTEM32\ntdll.dll+0x180052650) previously allocated by thread T0 here: #0 0x7fface90c432 in _asan_wrap_GlobalSize+0x49777 (C:\Users\Dim\Source\php-src\x64\Debug_TS\clang_rt.asan_dbg_dynamic-x86_64.dll+0x18004c432) #1 0x7ffacc749005 in allocate_new_resource C:\Users\Dim\Source\php-src\TSRM\TSRM.c:371 #2 0x7ffacc74717c in ts_resource_ex C:\Users\Dim\Source\php-src\TSRM\TSRM.c:445 #3 0x7ffacc67a97a in php_tsrm_startup C:\Users\Dim\Source\php-src\main\main.c:2680 #4 0x7ffb0350d3ad in php_apache_server_startup C:\Users\Dim\Source\php-src\sapi\apache2handler\sapi_apache2.c:481 #5 0x7ffb032bf0fd in ap_run_post_config+0x4d (C:\Apache24\bin\libhttpd.dll+0x18000f0fd) #6 0x7ff7c2111e50 in OPENSSL_Applink+0xe50 (C:\Apache24\bin\httpd.exe+0x140001e50) #7 0x7ff7c2112fa3 in OPENSSL_Applink+0x1fa3 (C:\Apache24\bin\httpd.exe+0x140002fa3) #8 0x7ffb1cd57033 in BaseThreadInitThunk+0x13 (C:\Windows\System32\KERNEL32.DLL+0x180017033) #9 0x7ffb1eaa2650 in RtlUserThreadStart+0x20 (C:\Windows\SYSTEM32\ntdll.dll+0x180052650) SUMMARY: AddressSanitizer: heap-use-after-free C:\Users\Dim\Source\php-src\Zend\zend_virtual_cwd.c:346 in realpath_cache_clean Shadow bytes around the buggy address: 0x04df91ec0770: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x04df91ec0780: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x04df91ec0790: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x04df91ec07a0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x04df91ec07b0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd =>0x04df91ec07c0: fd[fd]fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x04df91ec07d0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x04df91ec07e0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x04df91ec07f0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x04df91ec0800: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x04df91ec0810: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 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 Address Sanitizer Error: Use of deallocated memory It appears that each time cwd_globals_ctor() is called, a fresh realpath_cache is created, using virtual_cwd_globals *cwd_g as storage, so it is logical to use the same pointer in cwd_globals_dtor() to cleanup that storage. What fixed it for me was to copy the code in realpath_cache_clean() into cwd_globals_dtor(), but use cwd_g->xxx instead of CWDG(xxx), e.g.: static void cwd_globals_dtor(virtual_cwd_globals *cwd_g) /* {{{ */ { uint32_t i; for (i = 0; i < sizeof(cwd_g->realpath_cache)/sizeof(cwd_g->realpath_cache[0]); i++) { realpath_cache_bucket *p = cwd_g->realpath_cache[i]; while (p != NULL) { realpath_cache_bucket *r = p; p = p->next; free(r); } cwd_g->realpath_cache[i] = NULL; } cwd_g->realpath_cache_size = 0; } /* }}} */ However, this duplicates code so it is not really nice. Unfortunately you cannot use the CWDG() macro to get to the actual virtual_cwd_globals pointer, since zend.h does not have a ZEND_TSRMG_FAST_BULK() macro. So I think the second best is to have a helper function that takes the maximum number of entries, a pointer to the start of the realpath_cache array, and a pointer to the realpath_cache_size member. This helper function can then be called from both cwd_globals_dtor() and the externally available realpath_cache_clean(), e.g.: static void realpath_cache_clean_helper(uint32_t max_entries, realpath_cache_bucket **cache, zend_long *cache_size) { uint32_t i; for (i = 0; i < max_entries; i++) { realpath_cache_bucket *p = cache[i]; while (p != NULL) { realpath_cache_bucket *r = p; p = p->next; free(r); } cache[i] = NULL; } *cache_size = 0; } static void cwd_globals_dtor(virtual_cwd_globals *cwd_g) /* {{{ */ { realpath_cache_clean_helper(sizeof(cwd_g->realpath_cache)/sizeof(cwd_g->realpath_cache[0]), cwd_g->realpath_cache, &cwd_g->realpath_cache_size); } /* }}} */ [...] CWD_API void realpath_cache_clean(void) /* {{{ */ { realpath_cache_clean_helper(sizeof(CWDG(realpath_cache))/sizeof(CWDG(realpath_cache)[0]), CWDG(realpath_cache), &CWDG(realpath_cache_size)); } /* }}} */ See also the attached patch. There is probably a nicer way, but it would involve adding something like ZEND_TSRMG_FAST_BULK() to zend.h. Test script: --------------- See description above. Effectively: - Unzip https://windows.php.net/downloads/releases/php-8.0.6-Win32-vs16-x64.zip to C:\PHP80 - Unzip https://www.apachelounge.com/download/VS16/binaries/httpd-2.4.47-win64-VS16.zip to C:\Apache24 - Add lines to C:\Apache24\conf\httpd.conf: LoadModule php_module "C:/PHP80/php8apache2_4.dll" AddHandler application/x-httpd-php .php PHPIniDir "C:/PHP80" - Run C:\Apache24\bin\httpd.exe -X (in a separate terminal) - Run curl http://localhost/ - Control-C the httpd.exe - httpd.exe segfaults - usually this is visible in Event Viewer, but if you have Visual Studio installed you can start the just-in-time debugger) ------------------------------------------------------------------------ -- Edit this bug report at https://bugs.php.net/bug.php?id=81068&edit=1