From a8cd8000fe5814302758a26f4ad4fd9d392c91e0 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sun, 16 Apr 2023 15:05:03 +0200 Subject: [PATCH] Fix missing randomness check and insufficient random bytes for SOAP HTTP Digest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If php_random_bytes_throw fails, the nonce will be uninitialized, but still sent to the server. The client nonce is intended to protect against a malicious server. See section 5.10 and 5.12 of RFC 7616 [1], and bullet point 2 below. Tim pointed out that even though it's the MD5 of the nonce that gets sent, enumerating 31 bits is trivial. So we have still a stack information leak of 31 bits. Furthermore, Tim found the following issues: * The small size of cnonce might cause the server to erroneously reject a request due to a repeated (cnonce, nc) pair. As per the birthday problem 31 bits of randomness will return a duplication with 50% chance after less than 55000 requests and nc always starts counting at 1. * The cnonce is intended to protect the client and password against a malicious server that returns a constant server nonce where the server precomputed a rainbow table between passwords and correct client response. As storage is fairly cheap, a server could precompute the client responses for (a subset of) client nonces and still have a chance of reversing the client response with the same probability as the cnonce duplication. Precomputing the rainbow table for all 2^31 cnonces increases the rainbow table size by factor 2 billion, which is infeasible. But precomputing it for 2^14 cnonces only increases the table size by factor 16k and the server would still have a 10% chance of successfully reversing a password with a single client request. This patch fixes the issues by increasing the nonce size, and checking the return value of php_random_bytes_throw(). In the process we also get rid of the MD5 hashing of the nonce. [1] RFC 7616: https://www.rfc-editor.org/rfc/rfc7616 Co-authored-by: Tim Düsterhus (cherry picked from commit 126d517ce240e9f638d9a5eaa509eaca49ef562a) (cherry picked from commit 0cfca9aa1395271833848daec0bace51d965531d) --- NEWS | 6 ++++++ ext/soap/php_http.c | 21 +++++++++++++-------- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/NEWS b/NEWS index 47e9f89a646..ae5101b368e 100644 --- a/NEWS +++ b/NEWS @@ -1,6 +1,12 @@ PHP NEWS ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||| +Backported from 8.0.29 + +- Soap: + . Fixed bug GHSA-76gg-c692-v2mw (Missing error check and insufficient random + bytes in HTTP Digest authentication for SOAP). (nielsdos, timwolla) + Backported from 8.0.28 - Core: diff --git a/ext/soap/php_http.c b/ext/soap/php_http.c index 57754021b77..15b086e21c4 100644 --- a/ext/soap/php_http.c +++ b/ext/soap/php_http.c @@ -665,18 +665,23 @@ int make_http_soap_request(zval *this_ptr, if ((digest = zend_hash_str_find(Z_OBJPROP_P(this_ptr), "_digest", sizeof("_digest")-1)) != NULL) { if (Z_TYPE_P(digest) == IS_ARRAY) { char HA1[33], HA2[33], response[33], cnonce[33], nc[9]; - zend_long nonce; + unsigned char nonce[16]; PHP_MD5_CTX md5ctx; unsigned char hash[16]; - php_random_bytes_throw(&nonce, sizeof(nonce)); - nonce &= 0x7fffffff; + if (UNEXPECTED(php_random_bytes_throw(&nonce, sizeof(nonce)) != SUCCESS)) { + ZEND_ASSERT(EG(exception)); + php_stream_close(stream); + zend_hash_str_del(Z_OBJPROP_P(this_ptr), "httpurl", sizeof("httpurl")-1); + zend_hash_str_del(Z_OBJPROP_P(this_ptr), "httpsocket", sizeof("httpsocket")-1); + zend_hash_str_del(Z_OBJPROP_P(this_ptr), "_use_proxy", sizeof("_use_proxy")-1); + smart_str_free(&soap_headers_z); + smart_str_free(&soap_headers); + return FALSE; + } - PHP_MD5Init(&md5ctx); - snprintf(cnonce, sizeof(cnonce), ZEND_LONG_FMT, nonce); - PHP_MD5Update(&md5ctx, (unsigned char*)cnonce, strlen(cnonce)); - PHP_MD5Final(hash, &md5ctx); - make_digest(cnonce, hash); + php_hash_bin2hex(cnonce, nonce, sizeof(nonce)); + cnonce[32] = 0; if ((tmp = zend_hash_str_find(Z_ARRVAL_P(digest), "nc", sizeof("nc")-1)) != NULL && Z_TYPE_P(tmp) == IS_LONG) { From 1563873cd3f1fbd2464d3521b699f14efce1db13 Mon Sep 17 00:00:00 2001 From: Remi Collet Date: Tue, 6 Jun 2023 18:05:22 +0200 Subject: [PATCH] Fix GH-11382 add missing hash header for bin2hex (cherry picked from commit 40439039c224bb8cdebd1b7b3d03b8cc11e7cce7) --- ext/soap/php_http.c | 1 + 1 file changed, 1 insertion(+) diff --git a/ext/soap/php_http.c b/ext/soap/php_http.c index 15b086e21c..6903a3b9c9 100644 --- a/ext/soap/php_http.c +++ b/ext/soap/php_http.c @@ -23,6 +23,7 @@ #include "ext/standard/base64.h" #include "ext/standard/md5.h" #include "ext/standard/php_random.h" +#include "ext/hash/php_hash.h" static char *get_http_header_value(char *headers, char *type); static zend_string *get_http_body(php_stream *socketd, int close, char *headers); From 24d822d4e70431cc6dc795f7ff5193960f385c2f Mon Sep 17 00:00:00 2001 From: Remi Collet Date: Thu, 15 Jun 2023 08:47:55 +0200 Subject: [PATCH] add cve (cherry picked from commit f3021d66d7bb42d2578530cc94f9bde47e58eb10) --- NEWS | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index ae5101b368..5f49a7ee04 100644 --- a/NEWS +++ b/NEWS @@ -5,7 +5,8 @@ Backported from 8.0.29 - Soap: . Fixed bug GHSA-76gg-c692-v2mw (Missing error check and insufficient random - bytes in HTTP Digest authentication for SOAP). (nielsdos, timwolla) + bytes in HTTP Digest authentication for SOAP). + (CVE-2023-3247) (nielsdos, timwolla) Backported from 8.0.28 -- 2.40.1