From 9030bb04314b82877b01a8866c8574ee625676a7 Mon Sep 17 00:00:00 2001 From: David Carlier Date: Tue, 29 Apr 2025 22:42:43 +0100 Subject: [PATCH 1/4] Fixed GH-18458: `Authorization` set with CURLOPT_USERPWD with NULL value. --- ext/curl/interface.c | 18 +++++++++++++++++- ext/curl/tests/gh18458.phpt | 21 +++++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) create mode 100644 ext/curl/tests/gh18458.phpt diff --git a/ext/curl/interface.c b/ext/curl/interface.c index fe647dbafd4de..dd4e0db3d77b6 100644 --- a/ext/curl/interface.c +++ b/ext/curl/interface.c @@ -1900,7 +1900,6 @@ static zend_result _php_curl_setopt(php_curl *ch, zend_long option, zval *zvalue case CURLOPT_SSLKEYTYPE: case CURLOPT_SSL_CIPHER_LIST: case CURLOPT_USERAGENT: - case CURLOPT_USERPWD: case CURLOPT_COOKIELIST: case CURLOPT_FTP_ALTERNATIVE_TO_USER: case CURLOPT_SSH_HOST_PUBLIC_KEY_MD5: @@ -1998,6 +1997,23 @@ static zend_result _php_curl_setopt(php_curl *ch, zend_long option, zval *zvalue return ret; } + case CURLOPT_USERPWD: + { + if (Z_ISNULL_P(zvalue)) { + // Authorization header would be implictly set + // with an empty string thus we explictly set the option + // to null to avoid this unwarranted side effect + error = curl_easy_setopt(ch->cp, option, NULL); + } else { + zend_string *tmp_str; + zend_string *str = zval_get_tmp_string(zvalue, &tmp_str); + zend_result ret = php_curl_option_str(ch, option, ZSTR_VAL(str), ZSTR_LEN(str)); + zend_tmp_string_release(tmp_str); + return ret; + } + break; + } + /* Curl nullable string options */ case CURLOPT_CUSTOMREQUEST: case CURLOPT_FTPPORT: diff --git a/ext/curl/tests/gh18458.phpt b/ext/curl/tests/gh18458.phpt new file mode 100644 index 0000000000000..b692af4a0728f --- /dev/null +++ b/ext/curl/tests/gh18458.phpt @@ -0,0 +1,21 @@ +--TEST-- +GH-18458 authorization header is set despite CURLOPT_USERPWD set to null +--EXTENSIONS-- +curl +--SKIPIF-- + +--FILE-- + +--EXPECT-- +%A +bool(false) From bf9c881867534bafc59941bdc205f41d1849cdfc Mon Sep 17 00:00:00 2001 From: David Carlier Date: Wed, 30 Apr 2025 19:18:24 +0100 Subject: [PATCH 2/4] add CURLOPT_USERNAME case --- ext/curl/interface.c | 2 +- ext/curl/tests/gh18458.phpt | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/ext/curl/interface.c b/ext/curl/interface.c index dd4e0db3d77b6..a9e20aed87c7f 100644 --- a/ext/curl/interface.c +++ b/ext/curl/interface.c @@ -1906,7 +1906,6 @@ static zend_result _php_curl_setopt(php_curl *ch, zend_long option, zval *zvalue case CURLOPT_PASSWORD: case CURLOPT_PROXYPASSWORD: case CURLOPT_PROXYUSERNAME: - case CURLOPT_USERNAME: case CURLOPT_NOPROXY: case CURLOPT_SOCKS5_GSSAPI_SERVICE: case CURLOPT_MAIL_FROM: @@ -1998,6 +1997,7 @@ static zend_result _php_curl_setopt(php_curl *ch, zend_long option, zval *zvalue } case CURLOPT_USERPWD: + case CURLOPT_USERNAME: { if (Z_ISNULL_P(zvalue)) { // Authorization header would be implictly set diff --git a/ext/curl/tests/gh18458.phpt b/ext/curl/tests/gh18458.phpt index b692af4a0728f..d397e6e3b2323 100644 --- a/ext/curl/tests/gh18458.phpt +++ b/ext/curl/tests/gh18458.phpt @@ -15,7 +15,15 @@ curl_setopt($ch, CURLOPT_VERBOSE, true); curl_setopt($ch, CURLOPT_RETURNTRANSFER, 1); $response = curl_exec($ch); var_dump(str_contains($response, "authorization")); +$ch = curl_init("https://localhost/username"); +curl_setopt($ch, CURLOPT_USERNAME, null); +curl_setopt($ch, CURLOPT_VERBOSE, true); +curl_setopt($ch, CURLOPT_RETURNTRANSFER, 1); +$response = curl_exec($ch); +var_dump(str_contains($response, "authorization")); ?> --EXPECT-- %A bool(false) +%A +bool(false) From 1c25577be36eb1854312a45834f6d2be96c41943 Mon Sep 17 00:00:00 2001 From: David Carlier Date: Wed, 30 Apr 2025 19:41:51 +0100 Subject: [PATCH 3/4] fix test attempt --- ext/curl/tests/gh18458.phpt | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/ext/curl/tests/gh18458.phpt b/ext/curl/tests/gh18458.phpt index d397e6e3b2323..45d6155d4f020 100644 --- a/ext/curl/tests/gh18458.phpt +++ b/ext/curl/tests/gh18458.phpt @@ -12,17 +12,20 @@ include 'skipif-nocaddy.inc'; $ch = curl_init("https://localhost/userpwd"); curl_setopt($ch, CURLOPT_USERPWD, null); curl_setopt($ch, CURLOPT_VERBOSE, true); -curl_setopt($ch, CURLOPT_RETURNTRANSFER, 1); +curl_setopt($ch, CURLOPT_RETURNTRANSFER, true); +curl_setopt($ch, CURLOPT_STDERR, fopen("php://stdout", "w")); $response = curl_exec($ch); var_dump(str_contains($response, "authorization")); + $ch = curl_init("https://localhost/username"); curl_setopt($ch, CURLOPT_USERNAME, null); curl_setopt($ch, CURLOPT_VERBOSE, true); -curl_setopt($ch, CURLOPT_RETURNTRANSFER, 1); +curl_setopt($ch, CURLOPT_RETURNTRANSFER, true); +curl_setopt($ch, CURLOPT_STDERR, fopen("php://stdout", "w")); $response = curl_exec($ch); var_dump(str_contains($response, "authorization")); ?> ---EXPECT-- +--EXPECTF-- %A bool(false) %A From a7b60088a1a266aabf49fe1c72235492aa671264 Mon Sep 17 00:00:00 2001 From: David Carlier Date: Thu, 1 May 2025 13:36:31 +0100 Subject: [PATCH 4/4] reuse existing nullable option section. and add CURLOPT_PASSWORD to the mix. CURLOPT_XOAUTH2_BEARER already handled. --- ext/curl/interface.c | 25 ++++++------------------- ext/curl/tests/gh18458.phpt | 3 ++- 2 files changed, 8 insertions(+), 20 deletions(-) diff --git a/ext/curl/interface.c b/ext/curl/interface.c index a9e20aed87c7f..1a270a1c32cea 100644 --- a/ext/curl/interface.c +++ b/ext/curl/interface.c @@ -1903,7 +1903,6 @@ static zend_result _php_curl_setopt(php_curl *ch, zend_long option, zval *zvalue case CURLOPT_COOKIELIST: case CURLOPT_FTP_ALTERNATIVE_TO_USER: case CURLOPT_SSH_HOST_PUBLIC_KEY_MD5: - case CURLOPT_PASSWORD: case CURLOPT_PROXYPASSWORD: case CURLOPT_PROXYUSERNAME: case CURLOPT_NOPROXY: @@ -1996,24 +1995,6 @@ static zend_result _php_curl_setopt(php_curl *ch, zend_long option, zval *zvalue return ret; } - case CURLOPT_USERPWD: - case CURLOPT_USERNAME: - { - if (Z_ISNULL_P(zvalue)) { - // Authorization header would be implictly set - // with an empty string thus we explictly set the option - // to null to avoid this unwarranted side effect - error = curl_easy_setopt(ch->cp, option, NULL); - } else { - zend_string *tmp_str; - zend_string *str = zval_get_tmp_string(zvalue, &tmp_str); - zend_result ret = php_curl_option_str(ch, option, ZSTR_VAL(str), ZSTR_LEN(str)); - zend_tmp_string_release(tmp_str); - return ret; - } - break; - } - /* Curl nullable string options */ case CURLOPT_CUSTOMREQUEST: case CURLOPT_FTPPORT: @@ -2037,6 +2018,12 @@ static zend_result _php_curl_setopt(php_curl *ch, zend_long option, zval *zvalue case CURLOPT_HSTS: #endif case CURLOPT_KRBLEVEL: + // Authorization header would be implictly set + // with an empty string thus we explictly set the option + // to null to avoid this unwarranted side effect + case CURLOPT_USERPWD: + case CURLOPT_USERNAME: + case CURLOPT_PASSWORD: { if (Z_ISNULL_P(zvalue)) { error = curl_easy_setopt(ch->cp, option, NULL); diff --git a/ext/curl/tests/gh18458.phpt b/ext/curl/tests/gh18458.phpt index 45d6155d4f020..702737ac369ba 100644 --- a/ext/curl/tests/gh18458.phpt +++ b/ext/curl/tests/gh18458.phpt @@ -1,5 +1,5 @@ --TEST-- -GH-18458 authorization header is set despite CURLOPT_USERPWD set to null +GH-18458 (authorization header is set despite CURLOPT_USERPWD set to null) --EXTENSIONS-- curl --SKIPIF-- @@ -19,6 +19,7 @@ var_dump(str_contains($response, "authorization")); $ch = curl_init("https://localhost/username"); curl_setopt($ch, CURLOPT_USERNAME, null); +curl_setopt($ch, CURLOPT_PASSWORD, null); curl_setopt($ch, CURLOPT_VERBOSE, true); curl_setopt($ch, CURLOPT_RETURNTRANSFER, true); curl_setopt($ch, CURLOPT_STDERR, fopen("php://stdout", "w"));