search

php / php-src

The PHP Interpreter
https://www.php.net
Other
37.94k stars 7.72k forks source link

PHP Fatal error triggers pointer being freed was not allocated and malloc: double free for ptr errors #11078

Closed lucasnetau closed 3 months ago

lucasnetau commented 1 year ago

Description

The following code:

https://gist.github.com/lucasnetau/244ba31f321307e06177f48582273e86

Resulted in this output:

php(95590,0x7ff847fe5680) malloc: double free for ptr 0x7fd2c0198000

php(95560,0x7ff847fe5680) malloc: *** error for object 0x7fc2e801800a: pointer being freed was not allocated

SIGABRT

To trigger the issue a PHP Fatal error (out of memory) needs to occur in a write to a custom streamWrapper, the test case forces this quickly (pointer issue occurs on each run, double free occurs on most runs but not all). Notably when CURLOPT_HTTPHEADER is set the double free occurs in libcurl, when CURLOPT_HTTPHEADER is not set then the point being freed error occurs.

Double Free backtrace:

* thread #1, queue = 'com.apple.main-thread', stop reason = signal SIGABRT
    frame #0: 0x00007ff80467022a libsystem_kernel.dylib`__pthread_kill + 10
libsystem_kernel.dylib`:
->  0x7ff80467022a <+10>: jae    0x7ff804670234            ; <+20>
    0x7ff80467022c <+12>: movq   %rax, %rdi
    0x7ff80467022f <+15>: jmp    0x7ff804669ce2            ; cerror_nocancel
    0x7ff804670234 <+20>: retq   
Target 0: (php) stopped.
(lldb) bt
* thread #1, queue = 'com.apple.main-thread', stop reason = signal SIGABRT
  * frame #0: 0x00007ff80467022a libsystem_kernel.dylib`__pthread_kill + 10
    frame #1: 0x00007ff8046a7f7b libsystem_pthread.dylib`pthread_kill + 263
    frame #2: 0x00007ff8045f1ca5 libsystem_c.dylib`abort + 123
    frame #3: 0x00007ff804507637 libsystem_malloc.dylib`malloc_vreport + 888
    frame #4: 0x00007ff80451c897 libsystem_malloc.dylib`malloc_zone_error + 178
    frame #5: 0x0000000101b920ce libcurl.4.dylib`Curl_close(datap=0x00007ff7bfefe9d0) at url.c:410:3 [opt]
    frame #6: 0x0000000101b565e0 libcurl.4.dylib`curl_easy_cleanup(data=0x0000000000000000) at easy.c:804:3 [opt]
    frame #7: 0x00000001000735dd php`curl_free_obj(object=0x0000000103bcd798) at interface.c:2839:2 [opt]
    frame #8: 0x000000010047b31e php`zend_objects_store_del(object=0x0000000103bcd798) at zend_objects_API.c:200:4 [opt]
    frame #9: 0x00000001003d26af php`zend_llist_destroy(l=0x0000000103bc70a8) at zend_llist.c:109:4 [opt]
    frame #10: 0x00000001003d2703 php`zend_llist_clean(l=0x0000000103bc70a8) at zend_llist.c:123:2 [opt]
    frame #11: 0x000000010007ab71 php`curl_multi_free_obj(object=<unavailable>) at multi.c:555:2 [opt]
    frame #12: 0x000000010047b167 php`zend_objects_store_free_object_storage(objects=<unavailable>, fast_shutdown=<unavailable>) at zend_objects_API.c:122:6 [opt]
    frame #13: 0x00000001003cf2f2 php`zend_shutdown_executor_values(fast_shutdown=<unavailable>) at zend_execute_API.c:399:2 [opt]
    frame #14: 0x00000001003cf3d8 php`shutdown_executor at zend_execute_API.c:416:2 [opt]
    frame #15: 0x00000001003df2bc php`zend_deactivate at zend.c:1258:2 [opt]
    frame #16: 0x000000010037a1e5 php`php_request_shutdown(dummy=0x0000000000000000) at main.c:1863:2 [opt]
    frame #17: 0x00000001004c41dc php`do_cli(argc=4, argv=0x0000600000c08c90) at php_cli.c:1135:3 [opt]
    frame #18: 0x00000001004c1e81 php`main(argc=4, argv=0x0000600000c08c90) at php_cli.c:1333:18 [opt]
    frame #19: 0x00007ff804375310 dyld`start + 2432
(lldb) f 5
warning: libcurl.4.dylib was compiled with optimization - stepping may behave oddly; variables may not be available.
frame #5: 0x0000000101b920ce libcurl.4.dylib`Curl_close(datap=0x00007ff7bfefe9d0) at url.c:410:3 [opt]
   407    up_free(data);
   408    Curl_safefree(data->state.buffer);
   409    Curl_dyn_free(&data->state.headerb);
-> 410    Curl_safefree(data->state.ulbuf);
   411    Curl_flush_cookies(data, TRUE);
   412    Curl_altsvc_save(data, data->asi, data->set.str[STRING_ALTSVC]);
   413    Curl_altsvc_cleanup(&data->asi);
(lldb) f 6
frame #6: 0x0000000101b565e0 libcurl.4.dylib`curl_easy_cleanup(data=0x0000000000000000) at easy.c:804:3 [opt]
   801      return;
   802  
   803    sigpipe_ignore(data, &pipe_st);
-> 804    Curl_close(&data);
   805    sigpipe_restore(&pipe_st);
   806  }
   807  
(lldb) f 7
warning: php was compiled with optimization - stepping may behave oddly; variables may not be available.
frame #7: 0x00000001000735dd php`curl_free_obj(object=0x0000000103bcd798) at interface.c:2839:2 [opt]
   2836         curl_easy_setopt(ch->cp, CURLOPT_HEADERFUNCTION, curl_write_nothing);
   2837         curl_easy_setopt(ch->cp, CURLOPT_WRITEFUNCTION, curl_write_nothing);
   2838 
-> 2839         curl_easy_cleanup(ch->cp);
   2840 
   2841         /* cURL destructors should be invoked only by last curl handle */
   2842         if (--(*ch->clone) == 0) {

Pointer being freed was not allocated:

* thread #1, queue = 'com.apple.main-thread', stop reason = signal SIGABRT
    frame #0: 0x00007ff80467022a libsystem_kernel.dylib`__pthread_kill + 10
libsystem_kernel.dylib`:
->  0x7ff80467022a <+10>: jae    0x7ff804670234            ; <+20>
    0x7ff80467022c <+12>: movq   %rax, %rdi
    0x7ff80467022f <+15>: jmp    0x7ff804669ce2            ; cerror_nocancel
    0x7ff804670234 <+20>: retq   
Target 0: (php) stopped.
(lldb) bt
* thread #1, queue = 'com.apple.main-thread', stop reason = signal SIGABRT
  * frame #0: 0x00007ff80467022a libsystem_kernel.dylib`__pthread_kill + 10
    frame #1: 0x00007ff8046a7f7b libsystem_pthread.dylib`pthread_kill + 263
    frame #2: 0x00007ff8045f1ca5 libsystem_c.dylib`abort + 123
    frame #3: 0x00007ff804507637 libsystem_malloc.dylib`malloc_vreport + 888
    frame #4: 0x00007ff80450a951 libsystem_malloc.dylib`malloc_report + 151
    frame #5: 0x00007ff804595860 libsystem_c.dylib`fclose + 140
    frame #6: 0x0000000100391f0b php`stream_resource_regular_dtor(rsrc=<unavailable>) at streams.c:1761:19 [opt]
    frame #7: 0x00000001003f539a php`zend_resource_dtor(res=<unavailable>) at zend_list.c:73:3 [opt]
    frame #8: 0x00000001003f56df php`zend_close_rsrc_list(ht=0x0000000100ed5c30) at zend_list.c:225:5 [opt]
    frame #9: 0x00000001003cf5dc php`zend_shutdown_executor_values(fast_shutdown=<unavailable>) at zend_execute_API.c:277:3 [opt]
    frame #10: 0x00000001003cfcd8 php`shutdown_executor at zend_execute_API.c:416:2 [opt]
    frame #11: 0x00000001003dfbbc php`zend_deactivate at zend.c:1258:2 [opt]
    frame #12: 0x000000010037aae5 php`php_request_shutdown(dummy=0x0000000000000000) at main.c:1863:2 [opt]
    frame #13: 0x00000001004c4adc php`do_cli(argc=2, argv=0x00006000002080e0) at php_cli.c:1135:3 [opt]
    frame #14: 0x00000001004c2781 php`main(argc=2, argv=0x00006000002080e0) at php_cli.c:1333:18 [opt]
    frame #15: 0x00007ff804375310 dyld`start + 2432
(lldb) f 6
warning: php was compiled with optimization - stepping may behave oddly; variables may not be available.
frame #6: 0x0000000100391f0b php`stream_resource_regular_dtor(rsrc=<unavailable>) at streams.c:1761:19 [opt]
   1758 {
   1759         php_stream *stream = (php_stream*)rsrc->ptr;
   1760         /* set the return value for pclose */
-> 1761         FG(pclose_ret) = php_stream_free(stream, PHP_STREAM_FREE_CLOSE | PHP_STREAM_FREE_RSRC_DTOR);
   1762 }
   1763 
   1764 static void stream_resource_persistent_dtor(zend_resource *rsrc)
(lldb) f 7 
frame #7: 0x00000001003f539a php`zend_resource_dtor(res=<unavailable>) at zend_list.c:73:3 [opt]
   70           ZEND_ASSERT(ld && "Unknown list entry type");
   71   
   72           if (ld->list_dtor_ex) {
-> 73                   ld->list_dtor_ex(&r);
   74           }
   75   }
   76

Double free was reported to curl via https://github.com/curl/curl/issues/10964, however since a subtle change in settings (CURLOPT_HTTPHEADER) triggers a crash in PHP I am posting both here.

With ext-curl compiled with PHP_CURL_DEBUG=0 I only see the DTOR being called once (DTOR CALLED, ch = *)

The comments in interface.c before where the crash occurs for Curl are interesting https://github.com/php/php-src/blob/master/ext/curl/interface.c#L2829-2838

PHP Version

PHP 8.2.5

Operating System

MacOS / Linux

cgzones commented 1 year ago

Might be a duplicate of #10670

iluuu1994 commented 1 year ago

Might be a duplicate of https://github.com/php/php-src/issues/10670

This isn't trying to use custom allocators.

I'm unable to trigger a crash, ASAN is also not showing anything. What CURL version are you using? I'm trying this on Linux.

lucasnetau commented 1 year ago

Latest Curl 8.0.1 is used. So far I've triggered the crash on both MacOS and Linux (Docker). The crash is timing dependant so the rate of filling the buffer may need to change depending on your network, it does appear that Curl needs to be past a certain point in the transfer stage for the issue to occur.

I'm away from my main computer for a week, I'll come back the. with a code dump from Linux.

Is there any other flags I should use for building or running php to help?

iluuu1994 commented 1 year ago

That would be great, thank you!

Is there any other flags I should use for building or running php to help?

You could try the --enable-address-sanitizer and --enable-undefined-sanitizer flags.

lucasnetau commented 1 year ago

Additional address sanitiser output on MacOS

Double free output

DTOR CALLED, ch = 1b06c000
=================================================================
==51504==ERROR: AddressSanitizer: attempting double-free on 0x631000014800 in thread T0:
    #0 0x1169e7019 in wrap_free+0xa9 (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x4b019)
    #1 0x115b6cee2 in Curl_close url.c:432
    #2 0x115b31ae8 in curl_easy_cleanup easy.c:804
    #3 0x10e8657ca in curl_free_obj interface.c:2840
    #4 0x111c2b3cb in zend_objects_store_del zend_objects_API.c:200
    #5 0x1111f28b1 in rc_dtor_func zend_variables.c:57
    #6 0x1111f2b3c in i_zval_ptr_dtor zend_variables.h:44
    #7 0x1111f28f4 in zval_ptr_dtor zend_variables.c:84
    #8 0x10e8b81fc in _php_curl_multi_cleanup_list multi.c:109
    #9 0x11115a2be in zend_llist_destroy zend_llist.c:109
    #10 0x11115a5d4 in zend_llist_clean zend_llist.c:123
    #11 0x10e8c8748 in curl_multi_free_obj multi.c:555
    #12 0x111c28d5b in zend_objects_store_free_object_storage zend_objects_API.c:122
    #13 0x111129469 in zend_shutdown_executor_values zend_execute_API.c:399
    #14 0x11112a9ab in shutdown_executor zend_execute_API.c:416
    #15 0x111203029 in zend_deactivate zend.c:1258
    #16 0x110d77981 in php_request_shutdown main.c:1863
    #17 0x11218b810 in do_cli php_cli.c:1135
    #18 0x11218573d in main php_cli.c:1333
    #19 0x7ff80437530f  (<unknown module>)

0x631000014800 is located 0 bytes inside of 65536-byte region [0x631000014800,0x631000024800)
freed by thread T0 here:
    #0 0x1169e7019 in wrap_free+0xa9 (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x4b019)
    #1 0x7ff80459585f in fclose+0x8b (libsystem_c.dylib:x86_64+0x2585f)
    #2 0x1169d8a4f in wrap_fclose+0x8f (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x3ca4f)
    #3 0x110e336d2 in _php_stream_free streams.c:475
    #4 0x110e4a609 in stream_resource_regular_dtor streams.c:1761
    #5 0x111318ddb in zend_resource_dtor zend_list.c:73
    #6 0x11131b09c in zend_close_rsrc_list zend_list.c:225
    #7 0x111122434 in zend_shutdown_executor_values zend_execute_API.c:277
    #8 0x11112a9ab in shutdown_executor zend_execute_API.c:416
    #9 0x111203029 in zend_deactivate zend.c:1258
    #10 0x110d77981 in php_request_shutdown main.c:1863
    #11 0x11218b810 in do_cli php_cli.c:1135
    #12 0x11218573d in main php_cli.c:1333
    #13 0x7ff80437530f  (<unknown module>)

previously allocated by thread T0 here:
    #0 0x1169e6ed0 in wrap_malloc+0xa0 (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x4aed0)
    #1 0x115b6b95a in Curl_readwrite transfer.c:1120
    #2 0x115b54b91 in multi_runsingle multi.c:2436
    #3 0x115b54186 in curl_multi_perform multi.c:2714
    #4 0x10e8bde53 in zif_curl_multi_exec multi.c:225
    #5 0x111852da5 in ZEND_DO_ICALL_SPEC_RETVAL_UNUSED_HANDLER zend_vm_execute.h:1250
    #6 0x1113c4462 in execute_ex zend_vm_execute.h:55823
    #7 0x1113c5820 in zend_execute zend_vm_execute.h:60396
    #8 0x11120e4c9 in zend_execute_scripts zend.c:1826
    #9 0x110d8218b in php_execute_script main.c:2542
    #10 0x11218944f in do_cli php_cli.c:964
    #11 0x11218573d in main php_cli.c:1333
    #12 0x7ff80437530f  (<unknown module>)

SUMMARY: AddressSanitizer: double-free (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x4b019) in wrap_free+0xa9
==51504==ABORTING
Abort trap: 6

pointer being freed was not allocated output

=================================================================
==51699==ERROR: AddressSanitizer: attempting free on address which was not malloc()-ed: 0x63100001480a in thread T0
    #0 0x116e93019 in wrap_free+0xa9 (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x4b019)
    #1 0x7ff80459585f in fclose+0x8b (libsystem_c.dylib:x86_64+0x2585f)
    #2 0x116e84a4f in wrap_fclose+0x8f (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x3ca4f)
    #3 0x1112df6d2 in _php_stream_free streams.c:475
    #4 0x1112f6609 in stream_resource_regular_dtor streams.c:1761
    #5 0x1117c4ddb in zend_resource_dtor zend_list.c:73
    #6 0x1117c709c in zend_close_rsrc_list zend_list.c:225
    #7 0x1115ce434 in zend_shutdown_executor_values zend_execute_API.c:277
    #8 0x1115d69ab in shutdown_executor zend_execute_API.c:416
    #9 0x1116af029 in zend_deactivate zend.c:1258
    #10 0x111223981 in php_request_shutdown main.c:1863
    #11 0x112637810 in do_cli php_cli.c:1135
    #12 0x11263173d in main php_cli.c:1333
    #13 0x7ff80437530f  (<unknown module>)

0x63100001480a is located 10 bytes inside of 65536-byte region [0x631000014800,0x631000024800)
allocated by thread T0 here:
    #0 0x116e92ed0 in wrap_malloc+0xa0 (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x4aed0)
    #1 0x11601795a in Curl_readwrite transfer.c:1120
    #2 0x116000b91 in multi_runsingle multi.c:2436
    #3 0x116000186 in curl_multi_perform multi.c:2714
    #4 0x10ed69e53 in zif_curl_multi_exec multi.c:225
    #5 0x111cfeda5 in ZEND_DO_ICALL_SPEC_RETVAL_UNUSED_HANDLER zend_vm_execute.h:1250
    #6 0x111870462 in execute_ex zend_vm_execute.h:55823
    #7 0x111871820 in zend_execute zend_vm_execute.h:60396
    #8 0x1116ba4c9 in zend_execute_scripts zend.c:1826
    #9 0x11122e18b in php_execute_script main.c:2542
    #10 0x11263544f in do_cli php_cli.c:964
    #11 0x11263173d in main php_cli.c:1333
    #12 0x7ff80437530f  (<unknown module>)

SUMMARY: AddressSanitizer: bad-free (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x4b019) in wrap_free+0xa9
==51699==ABORTING
Abort trap: 6
lucasnetau commented 1 year ago

Alpine Linux produces a different error:

Thread 1 "php" received signal SIGSEGV, Segmentation fault.
0x000056407cf66486 in _php_stream_seek (stream=<error reading variable: Cannot access memory at address 0x7ffdf5351c68>, offset=<error reading variable: Cannot access memory at address 0x7ffdf5351c60>, whence=<error reading variable: Cannot access memory at address 0x7ffdf5351c5c>) at /usr/src/php/main/streams/streams.c:1295
1295    /usr/src/php/main/streams/streams.c: No such file or directory.
(gdb) bt
#0  0x000056407cf66486 in _php_stream_seek (stream=<error reading variable: Cannot access memory at address 0x7ffdf5351c68>, offset=<error reading variable: Cannot access memory at address 0x7ffdf5351c60>, whence=<error reading variable: Cannot access memory at address 0x7ffdf5351c5c>) at /usr/src/php/main/streams/streams.c:1295
#1  0x000056407cf69926 in stream_cookie_seeker (cookie=0x7ff82d077800, position=0x7ffdf53520d8, whence=1) at /usr/src/php/main/streams/cast.c:109
#2  0x00007ff82de108fc in ?? () from /lib/ld-musl-x86_64.so.1
#3  0x0000000000000000 in ?? ()

From Debian leaks are detected

=================================================================
==414==ERROR: LeakSanitizer: detected memory leaks

Indirect leak of 32480 byte(s) in 3 object(s) allocated from:
    #0 0x7f05b994ce8f in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:145
    #1 0x7f05b8fdc3c8  (/usr/lib/x86_64-linux-gnu/libcurl.so.4+0x2d3c8)

Indirect leak of 6440 byte(s) in 1 object(s) allocated from:
    #0 0x7f05b994d037 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
    #1 0x7f05b8fd5d6f  (/usr/lib/x86_64-linux-gnu/libcurl.so.4+0x26d6f)

Indirect leak of 2736 byte(s) in 1 object(s) allocated from:
    #0 0x7f05b994d037 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
    #1 0x7f05b8fd704f  (/usr/lib/x86_64-linux-gnu/libcurl.so.4+0x2804f)

Indirect leak of 416 byte(s) in 1 object(s) allocated from:
    #0 0x7f05b994d037 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
    #1 0x7f05b8ff2ec8  (/usr/lib/x86_64-linux-gnu/libcurl.so.4+0x43ec8)
    #2 0x558e286c53dc in ZEND_DO_ICALL_SPEC_RETVAL_USED_HANDLER /usr/src/php/Zend/zend_vm_execute.h:1312
    #3 0x558e28966226 in execute_ex /usr/src/php/Zend/zend_vm_execute.h:56032
    #4 0x558e2898214d in zend_execute /usr/src/php/Zend/zend_vm_execute.h:60396
    #5 0x558e285a1598 in zend_execute_scripts /usr/src/php/Zend/zend.c:1826
    #6 0x558e282d8a32 in php_execute_script /usr/src/php/main/main.c:2542
    #7 0x558e28d39862 in do_cli /usr/src/php/sapi/cli/php_cli.c:964
    #8 0x558e28d3c2bb in main /usr/src/php/sapi/cli/php_cli.c:1333
    #9 0x7f05b83f4d09 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x23d09)

Indirect leak of 256 byte(s) in 4 object(s) allocated from:
    #0 0x7f05b994ce8f in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:145
    #1 0x7f05b8fc85ec  (/usr/lib/x86_64-linux-gnu/libcurl.so.4+0x195ec)

Indirect leak of 127 byte(s) in 2 object(s) allocated from:
    #0 0x7f05b994ce8f in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:145
    #1 0x7f05b8fdc4c1  (/usr/lib/x86_64-linux-gnu/libcurl.so.4+0x2d4c1)

Indirect leak of 100 byte(s) in 4 object(s) allocated from:
    #0 0x7f05b98fa817 in __interceptor_strdup ../../../../src/libsanitizer/asan/asan_interceptors.cpp:452
    #1 0x7f05b8fffba1  (/usr/lib/x86_64-linux-gnu/libcurl.so.4+0x50ba1)

Indirect leak of 96 byte(s) in 1 object(s) allocated from:
    #0 0x7f05b994d037 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
    #1 0x7f05b8fd707a  (/usr/lib/x86_64-linux-gnu/libcurl.so.4+0x2807a)

Indirect leak of 70 byte(s) in 2 object(s) allocated from:
    #0 0x7f05b98fa817 in __interceptor_strdup ../../../../src/libsanitizer/asan/asan_interceptors.cpp:452
    #1 0x7f05b901f975  (/usr/lib/x86_64-linux-gnu/libcurl.so.4+0x70975)

Indirect leak of 48 byte(s) in 1 object(s) allocated from:
    #0 0x7f05b994ce8f in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:145
    #1 0x7f05b8fc2265  (/usr/lib/x86_64-linux-gnu/libcurl.so.4+0x13265)

Indirect leak of 30 byte(s) in 2 object(s) allocated from:
    #0 0x7f05b98fa817 in __interceptor_strdup ../../../../src/libsanitizer/asan/asan_interceptors.cpp:452
    #1 0x7f05b901f956  (/usr/lib/x86_64-linux-gnu/libcurl.so.4+0x70956)

Indirect leak of 24 byte(s) in 1 object(s) allocated from:
    #0 0x7f05b994d037 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
    #1 0x7f05b8fdd1ef  (/usr/lib/x86_64-linux-gnu/libcurl.so.4+0x2e1ef)

Indirect leak of 16 byte(s) in 1 object(s) allocated from:
    #0 0x7f05b994d037 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
    #1 0x7f05b8fc1276  (/usr/lib/x86_64-linux-gnu/libcurl.so.4+0x12276)

Indirect leak of 12 byte(s) in 1 object(s) allocated from:
    #0 0x7f05b98fa817 in __interceptor_strdup ../../../../src/libsanitizer/asan/asan_interceptors.cpp:452
    #1 0x7f05b8fda46a  (/usr/lib/x86_64-linux-gnu/libcurl.so.4+0x2b46a)

Indirect leak of 12 byte(s) in 1 object(s) allocated from:
    #0 0x7f05b98fa817 in __interceptor_strdup ../../../../src/libsanitizer/asan/asan_interceptors.cpp:452
    #1 0x7f05b8fd7d24  (/usr/lib/x86_64-linux-gnu/libcurl.so.4+0x28d24)

Indirect leak of 1 byte(s) in 1 object(s) allocated from:
    #0 0x7f05b98fa817 in __interceptor_strdup ../../../../src/libsanitizer/asan/asan_interceptors.cpp:452
    #1 0x7f05b8fd7de2  (/usr/lib/x86_64-linux-gnu/libcurl.so.4+0x28de2)

Indirect leak of 1 byte(s) in 1 object(s) allocated from:
    #0 0x7f05b98fa817 in __interceptor_strdup ../../../../src/libsanitizer/asan/asan_interceptors.cpp:452
    #1 0x7f05b8fd8366  (/usr/lib/x86_64-linux-gnu/libcurl.so.4+0x29366)

SUMMARY: AddressSanitizer: 42865 byte(s) leaked in 28 allocation(s).
lucasnetau commented 1 year ago

Depending on where the out of memory error occurs it appears a different order of shutdown. With STREAM_DEBUG enabled.

No crash occurs when the Curl handle is closed first (DTOR CALLED before stream_free):

stream_alloc: STDIO:0x112a68100 persistent=(null)
stream_alloc: STDIO:0x112a68300 persistent=(null)
stream_alloc: STDIO:0x112a68500 persistent=(null)
stream_alloc: user-space:0x112a68700 persistent=(null)
stream_alloc: user-space:0x112a68800 persistent=(null)
*   Trying 34.193.132.77:80...
* Connected to httpbin.org (34.193.132.77) port 80 (#0)
> POST /post HTTP/1.1
Host: httpbin.org
Accept: */*
content-length: 100000000
user-agent:TestClient/1
Expect: 100-continue

PHP Fatal error:  Allowed memory size of 16777216 bytes exhausted at Zend/zend_string.h:249 (tried to allocate 7766032 bytes) in /tests/killcurl.php on line 75

Fatal error: Allowed memory size of 16777216 bytes exhausted at Zend/zend_string.h:249 (tried to allocate 7766032 bytes) in /tests/killcurl.php on line 75
* Closing connection 0
DTOR CALLED, ch = 12a6c000
stream_free: user-space:0x112a68800[fifo://113] in_free=0 opts=CALL_DTOR, RELEASE_STREAM, RSRC_DTOR
stream_free: user-space:0x112a68800[fifo://113] preserve_handle=0 release_cast=1 remove_rsrc=0
stream_free: user-space:0x112a68800[fifo://113] in_free=0 opts=CALL_DTOR, RELEASE_STREAM, RSRC_DTOR
stream_free: user-space:0x112a68800[fifo://113] preserve_handle=0 release_cast=1 remove_rsrc=0
closed stream reader
stream_free: user-space:0x112a68700[fifo://113] in_free=0 opts=CALL_DTOR, RELEASE_STREAM, RSRC_DTOR
stream_free: user-space:0x112a68700[fifo://113] preserve_handle=0 release_cast=1 remove_rsrc=0
closed stream writer
stream_free: STDIO:0x112a68500[php://stderr] in_free=0 opts=CALL_DTOR, RELEASE_STREAM, RSRC_DTOR
stream_free: STDIO:0x112a68500[php://stderr] preserve_handle=1 release_cast=0 remove_rsrc=0
stream_free: STDIO:0x112a68300[php://stdout] in_free=0 opts=CALL_DTOR, RELEASE_STREAM, RSRC_DTOR
stream_free: STDIO:0x112a68300[php://stdout] preserve_handle=1 release_cast=0 remove_rsrc=0
stream_free: STDIO:0x112a68100[php://stdin] in_free=0 opts=CALL_DTOR, RELEASE_STREAM, RSRC_DTOR
stream_free: STDIO:0x112a68100[php://stdin] preserve_handle=1 release_cast=0 remove_rsrc=0

Crash occurs if the input file stream is closed before the curl connection is closed (DTOR CALLED after stream_free)

stream_alloc: STDIO:0x110468100 persistent=(null)
stream_alloc: STDIO:0x110468300 persistent=(null)
stream_alloc: STDIO:0x110468500 persistent=(null)
stream_alloc: user-space:0x110468700 persistent=(null)
stream_alloc: user-space:0x110468800 persistent=(null)
*   Trying 34.193.132.77:80...
* Connected to httpbin.org (34.193.132.77) port 80 (#0)
> POST /post HTTP/1.1
Host: httpbin.org
Accept: */*
content-length: 100000000
user-agent:TestClient/1
Expect: 100-continue

< HTTP/1.1 100 Continue
PHP Fatal error:  Allowed memory size of 16777216 bytes exhausted at Zend/zend_string.h:152 (tried to allocate 6939768 bytes) in /tests/killcurl.php on line 57

Fatal error: Allowed memory size of 16777216 bytes exhausted at Zend/zend_string.h:152 (tried to allocate 6939768 bytes) in /tests/killcurl.php on line 57
stream_free: user-space:0x110468800[fifo://321] in_free=0 opts=CALL_DTOR, RELEASE_STREAM, RSRC_DTOR
stream_free: user-space:0x110468800[fifo://321] preserve_handle=0 release_cast=1 remove_rsrc=0
stream_free: user-space:0x110468800[fifo://321] in_free=0 opts=CALL_DTOR, RELEASE_STREAM, RSRC_DTOR
stream_free: user-space:0x110468800[fifo://321] preserve_handle=0 release_cast=1 remove_rsrc=0
closed stream reader
stream_free: user-space:0x110468700[fifo://321] in_free=0 opts=CALL_DTOR, RELEASE_STREAM, RSRC_DTOR
stream_free: user-space:0x110468700[fifo://321] preserve_handle=0 release_cast=1 remove_rsrc=0
closed stream writer
stream_free: STDIO:0x110468500[php://stderr] in_free=0 opts=CALL_DTOR, RELEASE_STREAM, RSRC_DTOR
stream_free: STDIO:0x110468500[php://stderr] preserve_handle=1 release_cast=0 remove_rsrc=0
stream_free: STDIO:0x110468300[php://stdout] in_free=0 opts=CALL_DTOR, RELEASE_STREAM, RSRC_DTOR
stream_free: STDIO:0x110468300[php://stdout] preserve_handle=1 release_cast=0 remove_rsrc=0
stream_free: STDIO:0x110468100[php://stdin] in_free=0 opts=CALL_DTOR, RELEASE_STREAM, RSRC_DTOR
stream_free: STDIO:0x110468100[php://stdin] preserve_handle=1 release_cast=0 remove_rsrc=0
DTOR CALLED, ch = 1046c000
=================================================================
==37427==ERROR: AddressSanitizer: attempting double-free on 0x631000014800 in thread T0:
...
lucasnetau commented 1 year ago

@iluuu1994 please let me know if you need anything additional. I haven't been able to crash reliably on Linux after compiling with debug enabled (Alpine Linux SIGSEGV, Debian I get leaks but no crash). MacOS version crashes reliably every 2-3 invocations of the script with SIGABRT.

lucasnetau commented 1 year ago

I haven't managed to get any further in working out what is going on. I did note that the release of the stream also generates very different debug logs between the shutdown and normally when calling fclose()

shutdown

stream_free: user-space:0x110468800[fifo://321] in_free=0 opts=CALL_DTOR, RELEASE_STREAM, RSRC_DTOR
stream_free: user-space:0x110468800[fifo://321] preserve_handle=0 release_cast=1 remove_rsrc=0
stream_free: user-space:0x110468800[fifo://321] in_free=0 opts=CALL_DTOR, RELEASE_STREAM, RSRC_DTOR
stream_free: user-space:0x110468800[fifo://321] preserve_handle=0 release_cast=1 remove_rsrc=0

fclose

stream_free: user-space:0x10e268800[fifo://842] in_free=0 opts=CALL_DTOR, RELEASE_STREAM
stream_free: user-space:0x10e268800[fifo://842] preserve_handle=0 release_cast=1 remove_rsrc=1
stream_free: user-space:0x10e268800[fifo://842] in_free=1 opts=CALL_DTOR, RELEASE_STREAM, RSRC_DTOR
stream_free: user-space:0x10e268800[fifo://842] in_free=0 opts=CALL_DTOR, RELEASE_STREAM, RSRC_DTOR
stream_free: user-space:0x10e268800[fifo://842] preserve_handle=0 release_cast=1 remove_rsrc=0
iluuu1994 commented 1 year ago

Hi @lucasnetau! Sorry, I've been a little busy the past few days. I'll look into this again soon.

iluuu1994 commented 1 year ago

I finally had a closer look. Unfortunately, I still cannot reproduce this issue locally. But like you say, this issue seems to be cause by zend_close_rsrc_list being called before zend_objects_store_free_object_storage. From my understanding, this is what happens:

I think this can be avoided by calling curl_easy_cleanup() in dtor_obj instead of free_obj. At this point I'm not completely sure if this has other side-effects, like using global CURL objects from other dtors on shutdown that may now be closed. But that already sounds pretty dangerous. @lucasnetau Would it be possible to test the following patch?

Edit: On second thought, I'm not sure this changes anything, as the curl_easy_cleanup() won't actually remove the resource. This would work only if we kept a pointer to the resource around and close that, as this would actually remove the resource from EG(regular_list). Another option might be to just duplicate FILE* before passing it to CURL.

diff --git a/ext/curl/interface.c b/ext/curl/interface.c
index 2dd4a7bf65..7d3910625d 100644
--- a/ext/curl/interface.c
+++ b/ext/curl/interface.c
@@ -408,7 +408,7 @@ PHP_MINIT_FUNCTION(curl)

    memcpy(&curl_object_handlers, &std_object_handlers, sizeof(zend_object_handlers));
    curl_object_handlers.offset = XtOffsetOf(php_curl, std);
-   curl_object_handlers.free_obj = curl_free_obj;
+   curl_object_handlers.dtor_obj = curl_free_obj;
    curl_object_handlers.get_gc = curl_get_gc;
    curl_object_handlers.get_constructor = curl_get_constructor;
    curl_object_handlers.clone_obj = curl_clone_obj;

The memory leaks should be addressed by GH-11231. It switches CURL to use emalloc which frees all allocated memory at the end of the request. This is because we use longjmp to abort the request on fatal errors, but this may also skip over corresponding free calls. There seem to be some issues in CI I'll have to look at.

lucasnetau commented 1 year ago

@iluuu1994 good news is the patch above does appear to solve the double free issue, now I get a normal fatal error then exit.

I have only been able to replicate this on MacOS. I originally thought I crashed Linux but I haven't been able to replicate the SIGABRT (I was able to get a SIGSEV in Alpine linux)

One thing to note is that the piece of memory that cURL is freeing is not the *FILE but the upload buffer which takes data from the file. I had originally posted this to https://github.com/curl/curl/issues/10964, somehow the two are linked but the cURL maintainers don't see how.

The bad new is the emalloc patches didn't fix the not allocated error, however after testing I saw you closed that PR:

==79532==ERROR: AddressSanitizer: attempting free on address which was not malloc()-ed: 0x000113eac000 in thread T0
    #0 0x10f9ce019 in wrap_free+0xa9 (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x4b019)
    #1 0x7ff80459585f in fclose+0x8b (libsystem_c.dylib:x86_64+0x2585f)
    #2 0x10f9bfa4f in wrap_fclose+0x8f (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x3ca4f)
    #3 0x109dfb9e2 in _php_stream_free streams.c:475
    #4 0x109e12919 in stream_resource_regular_dtor streams.c:1761
    #5 0x10a2e10eb in zend_resource_dtor zend_list.c:73
    #6 0x10a2e33ac in zend_close_rsrc_list zend_list.c:225
    #7 0x10a0ea744 in zend_shutdown_executor_values zend_execute_API.c:277
    #8 0x10a0f2cbb in shutdown_executor zend_execute_API.c:416
    #9 0x10a1cb339 in zend_deactivate zend.c:1258
    #10 0x109d3fc91 in php_request_shutdown main.c:1863
    #11 0x10b153b20 in do_cli php_cli.c:1135
    #12 0x10b14da4d in main php_cli.c:1333
    #13 0x7ff80437530f  (<unknown module>)

Address 0x000113eac000 is a wild pointer inside of access range of size 0x000000000001.
SUMMARY: AddressSanitizer: bad-free (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x4b019) in wrap_free+0xa9
lucasnetau commented 1 year ago

As part of object freeing, curl_easy_cleanup() is called which closes FILE* again

curl_easy_cleanup shouldn't be doing that because in curl_multi_free_obj() calls _php_curl_verify_handlers on each cURL handle before freeing which takes care of un-assigning the FILE* that has gone away

And from what traces show it is the upload buffer being free'd that causes the double free.

iluuu1994 commented 1 year ago

@lucasnetau Thanks for the hint! (I don't know the curl extension well) Now I'm no longer sure how this issue occurs at all... If zend_close_rsrc_list() always closes the FILE* first and marks the resource type as -1, then _php_curl_verify_handlers should always reset the ch->handlers.read->fp pointer . _php_curl_verify_handlers is also called in curl_free_obj so order shouldn't matter. It's hard to debug without being able to reproduce this.

Just to be sure, you don't have any other extensions enabled that might interact with the issue?

Edit: Oh, I missed the last part of your message (it is the upload buffer being free'd that causes the double free).

lucasnetau commented 1 year ago

Nothing out of the ordinary in terms of extensions. PHP is installed via homebrew on MacOS.

Yes, the crash line in libcurl is Curl_safefree(data->state.ulbuf); and that appears to be allocated in libcurl

In terms of replicating, it does appear to be timing dependant on how far curl gets with the transfer before the OOM. You can try adjusting the 500 value to something smaller (say 100) or larger depending on your internet speed fwrite($writeStream, str_repeat('x', 500));

Since I can replicate, is there any steps you want me to do to capture any more information?

iluuu1994 commented 1 year ago

@lucasnetau I whipped out my dusty old MacBook, I can reproduce the problem there. I'll have a look again soon.

lucasnetau commented 8 months ago

Hi @iluuu1994 , were you ever about to find out anything more with this issue?

iluuu1994 commented 8 months ago

Hi @lucasnetau. No, I don't think I ever had another look. I'm not very familiar with curl or the extension. I would be grateful if somebody else could have a look, maybe @adoy?

Girgias commented 3 months ago

@SakiTakamachi could you possibly have a look at this as you have a macOS machine?

SakiTakamachi commented 3 months ago

I'll try it some more, but I can't reproduce it on either Intel or Arm MacBooks...

lucasnetau commented 3 months ago

Hi @SakiTakamachi , I can still reliably cause this crash every 2-3 executions of the test script.

Fatal error: Allowed memory size of 16777216 bytes exhausted (tried to allocate 11109840 bytes) in /tests/killcurl.php on line 57
php(30218,0x7ff8505be700) malloc: double free for ptr 0x7fb4c0040000
php(30218,0x7ff8505be700) malloc: *** set a breakpoint in malloc_error_break to debug
Abort trap: 6

It is very timing dependant (ensuring the OOM occurs in the stream handler) and also bandwidth dependant.

This line can be adjusted, reducing 500 to a say 100 if you have a faster connection. I believe from memory you need to see the HTTP/1.1 100 Continue output before the error triggers.

fwrite($writeStream, str_repeat('x', 500));
SakiTakamachi commented 3 months ago

@lucasnetau Thanks, in my environment I was able to reproduce it by setting it to 1000. And so far this problem only occurs on my Arm processor. Is your environment also Arm?

lucasnetau commented 3 months ago

This was done on x86_64, pretty sure I also replicated on m1 arm however the results above are from Intel

SakiTakamachi commented 3 months ago

No matter how many times I try, I can't reproduce it on Intel...There may be other factors involved, but for now, I'll check the details on M2, which I was able to reproduce.

SakiTakamachi commented 3 months ago

There has been little progress since I have just set up a "debug environment that can be reproduced," but when I measured the number of loops, in my environment, two patterns always occur a crash around 2500 times and a crash around 11000 times.

The former is pointer being freed was not allocated, and the latter is double free. There is a correlation, at least in my environment, between the types of errors that occur and the number of times the loop runs before crashing.

lucasnetau commented 3 months ago

It depends on which line the OOM occurs, it needs to occur in the stream_read() which would make sense since that would be when cURL is reading data, stream_write would be on the PHP side.

If the crash is on line 57, which is within stream_read() I get the double free 

self::$buffers[$this->bufferKey] = substr(self::$buffers[$this->bufferKey], $written);

If I comment out this line to no set the header then I get pointer being freed was not allocated if it crashed in stream_read

curl_setopt($curl, CURLOPT_HTTPHEADER, ["content-length: 100000000","user-agent:TestClient/1",'Transfer-Encoding:']);
//->to
//curl_setopt($curl, CURLOPT_HTTPHEADER, ["content-length: 100000000","user-agent:TestClient/1",'Transfer-Encoding:']);

If it's on line 75 within stream_write() I only get an OOM, no crash

self::$buffers[$this->bufferKey] .= $data;
lucasnetau commented 3 months ago

@SakiTakamachi a more succinct reproducer based on the observation about the crash in stream_read. Only implemented the required stream functions and only opened a RO stream which with trigger an OOM when read.

For me you can change the constant DOUBLE_FREE from true to false to trigger the two different errors.

Have tested on both Intel and M1 ARM MacOS, same output on both.

<?php declare(strict_types=1);

/**
 * true = malloc: double free for ptr
 * false = malloc: *** error for object 0x: pointer being freed was not allocated
 */
const DOUBLE_FREE = true;

const MEM = 8388608;
ini_set('memory_limit', (string)MEM);

class CrashingFifo {

    /** @var resource|null Stream context (this is set by PHP) */
    public $context;

    function stream_open($path, $mode, $options, &$opened_path): bool
    {
        return true;
    }

    function stream_read(int $count): false|string|null
    {
        return str_repeat('x', MEM+1);
    }
}

stream_register_wrapper('fifo', CrashingFifo::class);
$readStream = fopen('fifo://1', 'r');

$curl = curl_init();
curl_setopt_array($curl, [
    CURLOPT_URL => 'http://httpbin.org/post',
    CURLOPT_PUT => true,
    CURLOPT_INFILE => $readStream,
    CURLOPT_CUSTOMREQUEST => 'POST',
    CURLOPT_VERBOSE => true,
]);

if (DOUBLE_FREE) {
    curl_setopt($curl, CURLOPT_HTTPHEADER,
        ["content-length: " . MEM, "user-agent:TestClient/1", 'Transfer-Encoding:']);
}

$multi = curl_multi_init();
curl_multi_add_handle($multi, $curl);

do {
    curl_multi_exec($multi, $still_running);
} while ($still_running);
SakiTakamachi commented 3 months ago

Cool, reproducibility is perfect.

SakiTakamachi commented 3 months ago

I've researched this issue, but I still don't have any idea what's causing it. Ilija's idea solves the double free problem, but I don't understand why curl tries to release the memory allocated on the stream side in the first place. I'm wondering if this strange behavior is also related to the unassigned error.

nielsdos commented 3 months ago

This reminds me of a mysqlnd bug with streams from some time ago, where the resource was freed before object destruction as well. The solution there is to remove the resource from the regular list after opening the stream and doing the cleanup ourselves.

If this were reproducible on Linux I could give it a debug session, but it seems it's only reproducible on macOS seemingly? Do we know why?

SakiTakamachi commented 3 months ago

At this time I don't know why this only happens on MacOS.

I will try to see if this can be reproduced in an Intel Linux environment.

lucasnetau commented 3 months ago

I haven't tried but could it be a difference between Clang on MacOS and gcc on Linux?

SakiTakamachi commented 3 months ago

Tried it on my Intel MacBook.

It is reproduced on Host OS (Mac OS). 

% clang -v
Apple clang version 14.0.3 (clang-1403.0.22.14.1)
Target: x86_64-apple-darwin23.4.0
Thread model: posix
InstalledDir: /Library/Developer/CommandLineTools/usr/bin

% curl -V
curl 8.4.0 (x86_64-apple-darwin23.0) libcurl/8.4.0 (SecureTransport) LibreSSL/3.3.6 zlib/1.2.12 nghttp2/1.58.0
Release-Date: 2023-10-11
Protocols: dict file ftp ftps gopher gophers http https imap imaps ldap ldaps mqtt pop3 pop3s rtsp smb smbs smtp smtps telnet tftp
Features: alt-svc AsynchDNS GSS-API HSTS HTTP2 HTTPS-proxy IPv6 Kerberos Largefile libz MultiSSL NTLM NTLM_WB SPNEGO SSL threadsafe UnixSockets

However, it does not reproduce on docker ubuntu on Intel MacBook. I've tried it with both gcc and clang (I also built curl from source and tried both patterns) but it doesn't reproduce.

# clang -v
Ubuntu clang version 14.0.0-1ubuntu1.1
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin
Found candidate GCC installation: /usr/bin/../lib/gcc/x86_64-linux-gnu/11
Selected GCC installation: /usr/bin/../lib/gcc/x86_64-linux-gnu/11
Candidate multilib: .;@m64
Selected multilib: .;@m64

# curl -V
curl 8.4.0 (x86_64-pc-linux-gnu) libcurl/8.4.0 OpenSSL/3.0.2 zlib/1.2.11 libpsl/0.21.0 (+libidn2/2.3.2)
Release-Date: 2023-10-11
Protocols: dict file ftp ftps gopher gophers http https imap imaps mqtt pop3 pop3s rtsp smb smbs smtp smtps telnet tftp
Features: alt-svc AsynchDNS HSTS HTTPS-proxy IPv6 Largefile libz NTLM PSL SSL threadsafe TLS-SRP UnixSockets
SakiTakamachi commented 3 months ago

I forgot to write one thing. On my Intel MacBook I always end up with not allocated error. No double free errors occur.

edit: At Intel, I was building the master branch. M2 is 8.2. I will align the branches and test them tomorrow.

nielsdos commented 3 months ago

No need to keep looking for a Linux environment, I got my hands on an old mac system. From the Darwin version I see you're running Sonoma, so I'll give it a go tomorrow.

SakiTakamachi commented 3 months ago

Thx!

I tested the master branch on Arm and got a double free error.

nielsdos commented 3 months ago

I reproduced this on macOS Ventura. And damn, this issue gave me a headache. There's a lot to say here. TL;DR: This issue is completely unrelated to Curl, and 100% related to streams. And in theory this also affects BSD OSes.

The analysis posted in this thread so far isn't right. Curl doesn't contain the code to fclose the FILE handle. In fact, Curl doesn't even receive the handle to begin with. What it gets is a pointer to our php_curl data structure and a function to callback to perform the read. This function is curl_read and it is part of PHP. That function uses the file pointer.

For the description below, I will use the FreeBSD libc source code because that's what macOS is based on and I couldn't find the sources of Apple's libc, but low and behold based on what I saw when stepping through gdb this is still the same code between macOS and FreeBSD. EDIT: I found the sources of macOS and have updated this description.

Here's what actually happens: 1) We're creating a FILE handle from a stream using the casting mechanism. This will create a cookie-based FILE handle using funopen. 2) We're reading stream data using fread from the userspace stream. This will temporarily set a buffer into a field _bf.base here: https://github.com/apple-open-source-mirror/Libc/blob/5e566be7a7047360adfb35ffc44c6a019a854bea/stdio/FreeBSD/fread.c#L102-L103 This buffer is now equal to the upload buffer that Curl allocated and note that that buffer is owned by Curl. 3) The fatal error occurs and we bail out from the fread function, notice how the reset code is never executed and so the buffer will still point to Curl's upload buffer instead of FILE's own buffer: https://github.com/apple-open-source-mirror/Libc/blob/5e566be7a7047360adfb35ffc44c6a019a854bea/stdio/FreeBSD/fread.c#L117 4) The resources are destroyed, this includes our opened stream and because the FILE handle is cached, it gets destroyed as well. In fact, the stream code calls through fclose on purpose in this case. 5) The fclose code frees the _bs.base buffer: https://github.com/apple-open-source-mirror/Libc/blob/5e566be7a7047360adfb35ffc44c6a019a854bea/stdio/FreeBSD/fclose.c#L66-L67 However, this is not the buffer that FILE owns but the one that Curl owns because it isn't reset properly due to the bailout! 6) The objects are getting destroyed, and so the curl free logic is invoked. When Curl tries to gracefully clean up, it tries to free the buffer. But that buffer is actually already freed mistakingly by the C library!

This also explains why we can't reproduce it on Linux: this bizarre buffer swapperoo only happens on macOS and BSD, not on Linux.

You can easily prove this by testing this: https://gist.github.com/nielsdos/0d4f15f57ad984944249621e2670a50c

So why does it sometimes cause a use-after-free instead of double free? The reason you can change which kind of heap corruption triggers is because you're actually influencing the heap layout by setting an extra option, causing ASAN to draw different conclusions because the shadow bytes are slightly differently layed out.

As for the patch, Ilija's patch is not right: we should leave the object in a consistent state (e.g. the zend_object_std_dtor should definitely not be called in dtor_obj). Furthermore, when it bails out we will actually leak persistent memory because the dtor function is never called on fatal error. The fact that the dtor is not called is actually the reason the patch prevents the heap corruption, it just leaks instead. Also when native operating system resources (like actual file handles) are involved, those would be leaked as well. Also, that patch doesn't solve the fundamental problem that we're bailing out and not giving libc the chance to reset its state.

As an aside, I see there was a PR at one point to make Curl memory allocations use the request allocator, but it was closed because it isn't thread safe. I'd like to add that while you could enable it in ZTS, but nothing prevents third party extensions or libraries underlying to those extensions from using Curl too and storing that data for longer than a request. That would also cause problems. (That's the reason I can't easily switch libxml to the request allocator.)

Giving Curl a unique FILE handle might work, but this is probably not an intended use-case and when multiple close operations of different FILE handles for the same stream, this might not work properly. I do have a PoC patch for this though but this isn't the right solution. This must be solved at the stream layer.

Another possible temporary solution is using php_stream_read instead of fread in the Curl callback, but that doesn't solve the fundamental problem so the same issue will exist in other places as well...

nielsdos commented 3 months ago

A possible proper fix at the stream layer is this I think: https://gist.github.com/nielsdos/b47a42bb29f2ababdba35b37eaa3d468 It works on my system and shows no regressions. It simply disables buffering of that FILE handle in case a cookie-based FILE is used. That way, the buffer swapping will never happen and it avoids this bug. Since the stream layer itself also has a buffering mechanism this shouldn't hurt performance.

@SakiTakamachi @lucasnetau Can you please test the patch I linked? It should apply cleanly on master. I was only able to test on x86-64 macOS Ventura.

SakiTakamachi commented 3 months ago

@nielsdos Amazing! I tried your patch on both Intel and Arm, against the master branch. Both seem to solve the problem.

lucasnetau commented 3 months ago

@nielsdos ,thank you very much for you investigation. I will test out the patch.

lucasnetau commented 3 months ago

@nielsdos I can confirm that this patch fixes the issue for me on both Intel and ARM tested against 8.3 src

nielsdos commented 3 months ago

Thanks, I've managed to create an isolated test and opened a PR, let's hope CI agrees too.

lucasnetau commented 3 months ago

In doing some benchmark testing with curl uploads via the stream I also discovered that this patch also fixed an issue I was seeing where cURL stopped uploading after stream_read() returned false on MacOS (Since returning an empty string causes cURL to think the upload is complete so you need to instead send a read error) [Note: I also use curl pause to stop and resume uploads as a flow control method if the buffer drains], now the upload works correctly even if the upload buffer gets drained.

In the testing I was able to do, cURL would resume however it never called the stream_read() leading to no data being transferred.

Performance wise, I don't see an variance between old and patched version POSTing a large file to a localhost server, both sat at around 1350 MB/s throughput

nielsdos commented 3 months ago

@lucasnetau Thanks for the results, fix will be in the next release.