Skip to content

Commit

Permalink
Fix missing randomness check and insufficient random bytes for SOAP H…
Browse files Browse the repository at this point in the history
…TTP Digest

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

Additionally:
* Fix GH-11382 add missing hash header for bin2hex
* Update NEWS

Co-authored-by: Tim Düsterhus <timwolla@php.net>
Co-authored-by: Remi Collet <remi@remirepo.net>
Co-authored-by: Pierrick Charron <pierrick@php.net>
  • Loading branch information
4 people authored and ramsey committed Jun 6, 2023
1 parent f9117eb commit 8648eba
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 8 deletions.
2 changes: 2 additions & 0 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ PHP NEWS
done). (peter279k)

- Soap:
. Fixed bug GHSA-76gg-c692-v2mw (Missing error check and insufficient random
bytes in HTTP Digest authentication for SOAP). (nielsdos, timwolla)
. Fixed bug GH-8426 (make test fail while soap extension build). (nielsdos)

- SPL:
Expand Down
22 changes: 14 additions & 8 deletions ext/soap/php_http.c
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,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_nodup(char *headers, char *type, size_t *len);
static char *get_http_header_value(char *headers, char *type);
Expand Down Expand Up @@ -657,18 +658,23 @@ int make_http_soap_request(zval *this_ptr,
has_authorization = 1;
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);
convert_to_null(Z_CLIENT_HTTPURL_P(this_ptr));
convert_to_null(Z_CLIENT_HTTPSOCKET_P(this_ptr));
convert_to_null(Z_CLIENT_USE_PROXY_P(this_ptr));
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) {
Expand Down

0 comments on commit 8648eba

Please sign in to comment.