Update interface.c #7376
Update interface.c #7376
Conversation
- Buffer overflow
|
Is it possible to add a test for this? (Or at least describe what issue this is fixing?) |
| @@ -1625,6 +1625,7 @@ static int curl_debug(CURL *cp, curl_infotype type, char *buf, size_t buf_len, v | |||
| if (type == CURLINFO_HEADER_OUT) { | |||
| if (ch->header.str) { | |||
| zend_string_release_ex(ch->header.str, 0); | |||
| ch->header.str = NULL; | |||
There was a problem hiding this comment.
Is the problematic case where buf_len == 0, which means that this releases ch->header.str, but due to the if in the following line, it will not be overwritten by a new buffer?
There was a problem hiding this comment.
I found this problem in versions 5.5-5.6. The script crashed with the message "zend_mm_heap is corrupted", but support for these versions was already completed, and I did not check new versions.
Then the fall was caused by freeing memory without resetting the string length to zero. As I understand it, the re-request caused an attempt to free memory that was no longer owned by the variable. But I'm not sure since I'm not an expert.
We have now switched to versions 7 and 8 and the same problem occurred. The script crashes steadily on the second request with the message "Bus error".
I will try to write some code to check for this error.
There was a problem hiding this comment.
Unfortunately, I still cannot localize the error and make an example. On a working program without "ch>header.str = NULL;" the script lives no more than 10 minutes.
I am attaching info from gdb. It looks like the problem does not occur when using the curl_debug function, but after in the garbage collector.
Assertion failed: (source_list->ptr == prop), function zend_ref_del_type_source, file /root/source/php/php-8.0.9-debug/Zend/zend_execute.c, line 3306.
(gdb) bt
#0 0x0000000803194b8c in thr_kill () from /lib/libc.so.7
#1 0x00000008032319eb in abort () from /lib/libc.so.7
#2 0x000000080321ad85 in __assert () from /lib/libc.so.7
#3 0x0000000000967113 in zend_ref_del_type_source (source_list=0x810d64d18, prop=0x80377b9c0) at /root/source/php/php-8.0.9-debug/Zend/zend_execute.c:3306
#4 0x0000000000a12e08 in zend_object_std_dtor (object=0x8106cd000) at /root/source/php/php-8.0.9-debug/Zend/zend_objects.c:67
#5 0x0000000000a1a1c1 in zend_objects_store_del (object=0x8106cd000) at /root/source/php/php-8.0.9-debug/Zend/zend_objects_API.c:193
#6 0x0000000000933e64 in rc_dtor_func (p=0x8106cd000) at /root/source/php/php-8.0.9-debug/Zend/zend_variables.c:57
#7 0x0000000000a13029 in i_zval_ptr_dtor (zval_ptr=0x803678578) at zend_variables.h:44
#8 0x0000000000a12e11 in zend_object_std_dtor (object=0x803678500) at /root/source/php/php-8.0.9-debug/Zend/zend_objects.c:70
#9 0x00000000009fcf60 in zend_gc_collect_cycles () at /root/source/php/php-8.0.9-debug/Zend/zend_gc.c:1584
#10 0x00000000009536dc in zif_gc_collect_cycles (execute_data=0x803615330, return_value=0x7fffffffca40) at /root/source/php/php-8.0.9-debug/Zend/zend_builtin_functions.c:93
#11 0x000000000096e71d in ZEND_DO_FCALL_BY_NAME_SPEC_RETVAL_UNUSED_HANDLER (execute_data=0x8036150a0) at zend_vm_execute.h:1460
#12 0x00000000009ea16c in execute_ex (ex=0x803615020) at zend_vm_execute.h:54331
#13 0x00000000009ea2a4 in zend_execute (op_array=0x80365d280, return_value=0x0) at zend_vm_execute.h:58871
#14 0x0000000000938446 in zend_execute_scripts (type=8, retval=0x0, file_count=3) at /root/source/php/php-8.0.9-debug/Zend/zend.c:1680
#15 0x00000000008a28de in php_execute_script (primary_file=0x7fffffffe730) at /root/source/php/php-8.0.9-debug/main/main.c:2524
#16 0x0000000000a26690 in do_cli (argc=4, argv=0x7fffffffe9e8) at /root/source/php/php-8.0.9-debug/sapi/cli/php_cli.c:949
#17 0x0000000000a274ec in main (argc=4, argv=0x7fffffffe9e8) at /root/source/php/php-8.0.9-debug/sapi/cli/php_cli.c:1337
There was a problem hiding this comment.
I didn't manage to reproduce anything either, but I found a related issue, which is fixed by 30e791e.
There was a problem hiding this comment.
I merged a variant of your fix in 8c292a2. Rather than nulling out header.str, it drops the empty string check to make sure the old pointer is always overwritten.
There was a problem hiding this comment.
I applied this fix on 8.0.10 and the problem disappeared completely. Thanks.
| @@ -3468,6 +3469,7 @@ static void curl_free_obj(zend_object *object) | |||
| zval_ptr_dtor(&ch->handlers.std_err); | |||
| if (ch->header.str) { | |||
| zend_string_release_ex(ch->header.str, 0); | |||
| ch->header.str = NULL; | |||
There was a problem hiding this comment.
This is unlikely to be necessary, as this function destroys ch and it cannot be used afterwards anymore.
There was a problem hiding this comment.
I'm not sure about the usefulness of this line. You, as a developer, know better whether need it here. My code works fine with it.
Buffer overflow on second request with large header with CURLINFO_HEADER_OUT set