From ee836f3995e4bfc5dcd03b55682497242c2a12d4 Mon Sep 17 00:00:00 2001 From: Eric Norris Date: Wed, 27 Nov 2024 15:12:23 -0500 Subject: [PATCH] address first round of PR comments - use hardcoded allow list for $share_options - use zval_try_get_long --- ext/curl/share.c | 39 ++++++++++++++----- ext/curl/tests/curl_persistent_share_003.phpt | 2 +- ext/curl/tests/curl_persistent_share_004.phpt | 2 +- 3 files changed, 31 insertions(+), 12 deletions(-) diff --git a/ext/curl/share.c b/ext/curl/share.c index bd242d57f2278..60106925c2900 100644 --- a/ext/curl/share.c +++ b/ext/curl/share.c @@ -160,20 +160,39 @@ PHP_FUNCTION(curl_share_init_persistent) ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(share_opts), entry) { ZVAL_DEREF(entry); - zend_ulong option = zval_get_long_ex(entry, true); - - if (option == CURL_LOCK_DATA_COOKIE) { - zend_throw_exception_ex( - NULL, - 0, - "CURL_LOCK_DATA_COOKIE is not allowed with persistent curl share handles" - ); + bool failed = false; + zend_ulong option = zval_try_get_long(entry, &failed); + if (failed) { + zend_argument_type_error(1, "must contain only longs, %s given", zend_zval_value_name(entry)); goto error; } - // Ensure that each additional option results in a unique persistent ID. - persistent_id += 1 << option; + switch (option) { + // Disallowed options + case CURL_LOCK_DATA_COOKIE: + zend_argument_value_error(1, "CURL_LOCK_DATA_COOKIE is not allowed"); + goto error; + + // Allowed options + case CURL_LOCK_DATA_DNS: + persistent_id |= 1 << 0; + break; + case CURL_LOCK_DATA_SSL_SESSION: + persistent_id |= 1 << 1; + break; + case CURL_LOCK_DATA_CONNECT: + persistent_id |= 1 << 2; + break; + case CURL_LOCK_DATA_PSL: + persistent_id |= 1 << 3; + break; + + // Unknown options + default: + zend_argument_value_error(1, "must contain only CURL_LOCK_DATA_* constants"); + goto error; + } } ZEND_HASH_FOREACH_END(); zend_array_sort(Z_ARRVAL_P(share_opts), php_array_data_compare_unstable_i, 1); diff --git a/ext/curl/tests/curl_persistent_share_003.phpt b/ext/curl/tests/curl_persistent_share_003.phpt index 99910bd80fdc7..a64440c11883b 100644 --- a/ext/curl/tests/curl_persistent_share_003.phpt +++ b/ext/curl/tests/curl_persistent_share_003.phpt @@ -9,7 +9,7 @@ $sh = curl_share_init_persistent([CURL_LOCK_DATA_DNS, CURL_LOCK_DATA_CONNECT, 30 ?> --EXPECTF-- -Fatal error: Uncaught Exception: Could not construct persistent cURL share handle: Unknown share option in %scurl_persistent_share_003.php:3 +Fatal error: Uncaught ValueError: curl_share_init_persistent(): Argument #1 ($share_options) must contain only CURL_LOCK_DATA_* constants in %scurl_persistent_share_003.php:3 Stack trace: #0 %scurl_persistent_share_003.php(3): curl_share_init_persistent(Array) #1 {main} diff --git a/ext/curl/tests/curl_persistent_share_004.phpt b/ext/curl/tests/curl_persistent_share_004.phpt index a16cb5123bb78..af1d7924375ea 100644 --- a/ext/curl/tests/curl_persistent_share_004.phpt +++ b/ext/curl/tests/curl_persistent_share_004.phpt @@ -9,7 +9,7 @@ $sh = curl_share_init_persistent([CURL_LOCK_DATA_COOKIE]); ?> --EXPECTF-- -Fatal error: Uncaught Exception: CURL_LOCK_DATA_COOKIE is not allowed with persistent curl share handles in %scurl_persistent_share_004.php:3 +Fatal error: Uncaught ValueError: curl_share_init_persistent(): Argument #1 ($share_options) CURL_LOCK_DATA_COOKIE is not allowed in %scurl_persistent_share_004.php:3 Stack trace: #0 %scurl_persistent_share_004.php(3): curl_share_init_persistent(Array) #1 {main}