Skip to content

openssl may modify member types of certificate arrays #16357

Closed
Closed
openssl may modify member types of certificate arrays#16357
Assignees
bukka
Labels
BugExtension: opensslStatus: Verified

Description

@YuanchengJiang
YuanchengJiang
opened on Oct 11, 2024

Description

The following code:

<?php
$signals = [SIGCONT];
$fusion = $signals;
$infile = __DIR__ . "/cert.crt";
$outfile = tempnam(sys_get_temp_dir(), "ssl");
var_dump(openssl_pkcs7_encrypt($infile, $outfile, $fusion, $headers, 0, $cipher));

Resulted in this output:

AddressSanitizer:DEADLYSIGNAL
=================================================================
==2185046==ERROR: AddressSanitizer: SEGV on unknown address 0x0000422c6710 (pc 0x558038e3c03a bp 0x7ffde93ffb30 sp 0x7ffde93ffab0 T0)
==2185046==The signal is caused by a WRITE memory access.
    #0 0x558038e3c03a in _try_convert_to_string /php-src/Zend/zend_operators.c:792:2
    #1 0x558034bb7bc2 in try_convert_to_string /php-src/Zend/zend_operators.h:374:9
    #2 0x558034b82c9c in php_openssl_x509_from_zval /php-src/ext/openssl/openssl.c:1527:7
    #3 0x558034b81581 in zif_openssl_pkcs7_encrypt /php-src/ext/openssl/openssl.c:5766:11
    #4 0x55803870eda3 in ZEND_DO_ICALL_SPEC_RETVAL_USED_HANDLER /php-src/Zend/zend_vm_execute.h:1363:2
    #5 0x558038243713 in execute_ex /php-src/Zend/zend_vm_execute.h:58565:7
    #6 0x558038245872 in zend_execute /php-src/Zend/zend_vm_execute.h:64217:2
    #7 0x558038f45b01 in zend_execute_script /php-src/Zend/zend.c:1928:3
    #8 0x558037856798 in php_execute_script_ex /php-src/main/main.c:2574:13
    #9 0x558037857858 in php_execute_script /php-src/main/main.c:2614:9
    #10 0x558038f59276 in do_cli /php-src/sapi/cli/php_cli.c:935:5
    #11 0x558038f53944 in main /php-src/sapi/cli/php_cli.c:1310:18
    #12 0x7f64f1907d8f  (/lib/x86_64-linux-gnu/libc.so.6+0x29d8f) (BuildId: 490fef8403240c91833978d494d39e537409b92e)
    #13 0x7f64f1907e3f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x29e3f) (BuildId: 490fef8403240c91833978d494d39e537409b92e)
    #14 0x558034806db4 in _start (/php-src/sapi/cli/php+0x2606db4) (BuildId: 10f0f1f073ebe7bcaf9944c9da559b10d59463b0)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /php-src/Zend/zend_operators.c:792:2 in _try_convert_to_string
==2185046==ABORTING

To reproduce:

-d "opcache.protect_memory=1" -d "zend_extension=/php-src/modules/opcache.so" -d "opcache.enable_cli=1"

PHP Version

nightly

Operating System

ubuntu 22.04

Activity

  1. YuanchengJiang
    added
    Bug
    Status: Needs Triage
    on Oct 11, 2024
  2. devnexen
    added
    Extension: openssl
    Status: Verified
    and removed
    Status: Needs Triage
    on Oct 11, 2024
  3. devnexen
    assigned
    bukka
    on Oct 11, 2024
  4. devnexen
    added
    Extension: opcache
    on Oct 11, 2024

  5. cmb69 commented on Oct 11, 2024

    @cmb69
    cmb69
    on Oct 11, 2024
    Member

    Simplified reproducer:

    $fusion = [123];
$infile = __DIR__ . "/php-src/ext/openssl/tests/cert.crt";
$outfile = tempnam(sys_get_temp_dir(), "ssl");
var_dump(openssl_pkcs7_encrypt($infile, $outfile, $fusion, null));
var_dump($fusion);

    outputs:

    bool(false)
array(1) {
  [0]=>
  string(3) "123"
}

    The argument $fusion has been modified, although not passed by reference. Something like

     ext/openssl/openssl.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/ext/openssl/openssl.c b/ext/openssl/openssl.c
index bc20647dd2..d4ba8664ba 100644
--- a/ext/openssl/openssl.c
+++ b/ext/openssl/openssl.c
@@ -1524,11 +1524,12 @@ static X509 *php_openssl_x509_from_zval(
 
 	*free_cert = 1;
 
-	if (!try_convert_to_string(val)) {
+	zend_string *str = zval_try_get_string(val);
+	if (str == NULL) {
 		return NULL;
 	}
 
-	return php_openssl_x509_from_str(Z_STR_P(val), arg_num, is_from_array, option_name);
+	return php_openssl_x509_from_str(str, arg_num, is_from_array, option_name);
 }
 /* }}} */

    might do.

  6. cmb69
    added a commit that references this issue on Oct 11, 2024

    Fix phpGH-16357: openssl may modify member types of certificate arrays

    6fdf09d
  7. cmb69
    linked a pull request that will close this issueFix GH-16357: openssl may modify member types of certificate arrays #16370on Oct 11, 2024

  8. cmb69 commented on Oct 11, 2024

    @cmb69
    cmb69
    on Oct 11, 2024
    Member

    This is not really related to OPcache (that only uncovers the problem due to a segfault if opcache.protect_memory=1).

  9. cmb69
    removed
    Extension: opcache
    on Oct 11, 2024
  10. cmb69
    changed the title [-]Segmentation fault in Zend/zend_operators.c:792[/-] [+]openssl may modify member types of certificate arrays[/+] on Oct 11, 2024

  11. cmb69 commented on Oct 11, 2024

    @cmb69
    cmb69
    on Oct 11, 2024
    Member

    Oh, forgot: thank you @YuanchengJiang for finding and reporting these issues!

  12. YuanchengJiang commented on Oct 11, 2024

    @YuanchengJiang
    YuanchengJiang
    on Oct 11, 2024
    Author

    my pleasure ;) thank you all for resolving these issues for better PHP

  13. cmb69
    added 2 commits that reference this issue on Oct 12, 2024

    Fix GH-16357: openssl may modify member types of certificate arrays

    549bcdb

    Merge branch 'PHP-8.2' into PHP-8.3

    76a819e
  14. cmb69
    closed this as completedin 549bcdbon Oct 12, 2024
  15. cmb69
    added 2 commits that reference this issue on Oct 12, 2024

    Merge branch 'PHP-8.3' into PHP-8.4

    33fab73

    Merge branch 'PHP-8.4'

    40052b3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

BugExtension: opensslStatus: Verified

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions