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

Segmentation fault in snmpget #16959

Closed
YuanchengJiang opened this issue Nov 27, 2024 · 4 comments
Closed

Segmentation fault in snmpget #16959

YuanchengJiang opened this issue Nov 27, 2024 · 4 comments

Comments

@YuanchengJiang
Copy link

Description

The following code:

<?php
$unsorted_oct_array = array (
077 => 077, -066 => -066, -0345 => -0345, 0 => 0
);
$fusion = $unsorted_oct_array;
var_dump((snmpget($hostname, $communityWrite, $fusion, $timeout, $retries) === $newvalue1));

Resulted in this output:

AddressSanitizer:DEADLYSIGNAL
=================================================================
==2528537==ERROR: AddressSanitizer: SEGV on unknown address 0x0000413a56f8 (pc 0x000004c2ec79 bp 0x7ffd64b211e0 sp 0x7ffd64b20d20 T0)
==2528537==The signal is caused by a WRITE memory access.
    #0 0x4c2ec79 in _convert_to_string /home/phpfuzz/WorkSpace/flowfusion/php-src/Zend/zend_operators.c:750:4
    #1 0x26a0123 in php_snmp_parse_oid /home/phpfuzz/WorkSpace/flowfusion/php-src/ext/snmp/snmp.c:682:4
    #2 0x2681e93 in php_snmp /home/phpfuzz/WorkSpace/flowfusion/php-src/ext/snmp/snmp.c:1247:7
    #3 0x26774a4 in zif_snmpget /home/phpfuzz/WorkSpace/flowfusion/php-src/ext/snmp/snmp.c:1310:2
    #4 0x44ba9ea in ZEND_DO_ICALL_SPEC_RETVAL_USED_HANDLER /home/phpfuzz/WorkSpace/flowfusion/php-src/Zend/zend_vm_execute.h:1363:2
    #5 0x3fb01c7 in execute_ex /home/phpfuzz/WorkSpace/flowfusion/php-src/Zend/zend_vm_execute.h:58595:7
    #6 0x3fb244c in zend_execute /home/phpfuzz/WorkSpace/flowfusion/php-src/Zend/zend_vm_execute.h:64247:2
    #7 0x4d48a09 in zend_execute_script /home/phpfuzz/WorkSpace/flowfusion/php-src/Zend/zend.c:1934:3
    #8 0x355e25a in php_execute_script_ex /home/phpfuzz/WorkSpace/flowfusion/php-src/main/main.c:2577:13
    #9 0x355f398 in php_execute_script /home/phpfuzz/WorkSpace/flowfusion/php-src/main/main.c:2617:9
    #10 0x4d5cd1a in do_cli /home/phpfuzz/WorkSpace/flowfusion/php-src/sapi/cli/php_cli.c:938:5
    #11 0x4d571ff in main /home/phpfuzz/WorkSpace/flowfusion/php-src/sapi/cli/php_cli.c:1313:18
    #12 0x7fb1c420ed8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
    #13 0x7fb1c420ee3f in __libc_start_main csu/../csu/libc-start.c:392:3
    #14 0x605a64 in _start (/home/phpfuzz/WorkSpace/flowfusion/php-src/sapi/cli/php+0x605a64)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /home/phpfuzz/WorkSpace/flowfusion/php-src/Zend/zend_operators.c:750:4 in _convert_to_string

To reproduce:

-d "opcache.protect_memory=1" -d "zend_extension=/home/phpfuzz/WorkSpace/flowfusion/php-src/modules/opcache.so" -d "opcache.enable_cli=1"

PHP Version

nightly

Operating System

ubuntu 22.04

@cmb69
Copy link
Member

cmb69 commented Nov 27, 2024

php-src/ext/snmp/snmp.c

Lines 681 to 682 in 17187c4

ZEND_HASH_FOREACH_VAL(oid_ht, tmp_oid) {
convert_to_string(tmp_oid);

We must not convert array elements when the array is passed by value. That's even a problem without OPcache, because we may change the array. Adding a var_dump($unsorted_oct_array); at the end of the script shows the conversion.

@devnexen
Copy link
Member

ah true, maybe zval_(try)_get_string is more appropriate here ?

@cmb69
Copy link
Member

cmb69 commented Nov 27, 2024

Yes, see e.g. #16370. If you want to work on fix, I'll gladly leave that to you. :)

@devnexen
Copy link
Member

Alright I ll have a look at it in few hours.

@cmb69 cmb69 assigned devnexen and unassigned cmb69 Nov 27, 2024
devnexen added a commit to devnexen/php-src that referenced this issue Nov 27, 2024
Instead of modifying the zval, we use the zend_try_get_string.
@devnexen devnexen linked a pull request Nov 27, 2024 that will close this issue
devnexen added a commit to devnexen/php-src that referenced this issue Dec 1, 2024
Instead of modifying the zval, we use the zend_try_get_string.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants