mirror of
https://github.com/php/php-src.git
synced 2024-09-21 09:57:23 +00:00
Fix missing randomness check and insufficient random bytes for SOAP HTTP 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>
This commit is contained in:
parent
f9117eb824
commit
8648eba93a
2
NEWS
2
NEWS
@ -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:
|
||||
|
@ -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);
|
||||
@ -657,18 +658,23 @@ try_again:
|
||||
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) {
|
||||
|
Loading…
Reference in New Issue
Block a user