From 0216630ea2815a5789a24279a1211ac398d4de79 Mon Sep 17 00:00:00 2001 From: Stanislav Malyshev Date: Sat, 26 Sep 2020 22:08:52 -0700 Subject: [PATCH 1/3] Fix bug #79601 (Wrong ciphertext/tag in AES-CCM encryption for a 12 bytes IV) --- ext/openssl/openssl.c | 10 ++++----- ext/openssl/tests/cipher_tests.inc | 21 +++++++++++++++++ ext/openssl/tests/openssl_decrypt_ccm.phpt | 22 +++++++++++------- ext/openssl/tests/openssl_encrypt_ccm.phpt | 26 ++++++++++++++-------- 4 files changed, 57 insertions(+), 22 deletions(-) diff --git a/ext/openssl/openssl.c b/ext/openssl/openssl.c index 6c1dd9d3435..3eea08c6490 100644 --- a/ext/openssl/openssl.c +++ b/ext/openssl/openssl.c @@ -6391,11 +6391,6 @@ static int php_openssl_validate_iv(char **piv, size_t *piv_len, size_t iv_requir { char *iv_new; - /* Best case scenario, user behaved */ - if (*piv_len == iv_required_len) { - return SUCCESS; - } - if (mode->is_aead) { if (EVP_CIPHER_CTX_ctrl(cipher_ctx, mode->aead_ivlen_flag, *piv_len, NULL) != 1) { php_error_docref(NULL, E_WARNING, "Setting of IV length for AEAD mode failed"); @@ -6404,6 +6399,11 @@ static int php_openssl_validate_iv(char **piv, size_t *piv_len, size_t iv_requir return SUCCESS; } + /* Best case scenario, user behaved */ + if (*piv_len == iv_required_len) { + return SUCCESS; + } + iv_new = ecalloc(1, iv_required_len + 1); if (*piv_len == 0) { diff --git a/ext/openssl/tests/cipher_tests.inc b/ext/openssl/tests/cipher_tests.inc index b1e46b411e5..779bfa8515c 100644 --- a/ext/openssl/tests/cipher_tests.inc +++ b/ext/openssl/tests/cipher_tests.inc @@ -1,5 +1,26 @@ array( + array( + 'key' => '404142434445464748494a4b4c4d4e4f', + 'iv' => '1011121314151617', + 'aad' => '000102030405060708090a0b0c0d0e0f', + 'tag' => '1fc64fbfaccd', + 'pt' => '202122232425262728292a2b2c2d2e2f', + 'ct' => 'd2a1f0e051ea5f62081a7792073d593d', + ), + array( + 'key' => '404142434445464748494a4b4c4d4e4f', + 'iv' => '101112131415161718191a1b', + 'aad' => '000102030405060708090a0b0c0d0e0f' . + '10111213', + 'tag' => '484392fbc1b09951', + 'pt' => '202122232425262728292a2b2c2d2e2f' . + '3031323334353637', + 'ct' => 'e3b201a9f5b71a7a9b1ceaeccd97e70b' . + '6176aad9a4428aa5', + ), + ), 'aes-256-ccm' => array( array( 'key' => '1bde3251d41a8b5ea013c195ae128b21' . diff --git a/ext/openssl/tests/openssl_decrypt_ccm.phpt b/ext/openssl/tests/openssl_decrypt_ccm.phpt index a5f01b87cea..08ef5bb7b7c 100644 --- a/ext/openssl/tests/openssl_decrypt_ccm.phpt +++ b/ext/openssl/tests/openssl_decrypt_ccm.phpt @@ -10,14 +10,16 @@ if (!in_array('aes-256-ccm', openssl_get_cipher_methods())) --FILE-- $test) { - echo "TEST $idx\n"; - $pt = openssl_decrypt($test['ct'], $method, $test['key'], OPENSSL_RAW_DATA, - $test['iv'], $test['tag'], $test['aad']); - var_dump($test['pt'] === $pt); +foreach ($methods as $method) { + $tests = openssl_get_cipher_tests($method); + foreach ($tests as $idx => $test) { + echo "$method - TEST $idx\n"; + $pt = openssl_decrypt($test['ct'], $method, $test['key'], OPENSSL_RAW_DATA, + $test['iv'], $test['tag'], $test['aad']); + var_dump($test['pt'] === $pt); + } } // no IV @@ -32,7 +34,11 @@ var_dump(openssl_decrypt($test['ct'], $method, $test['key'], OPENSSL_RAW_DATA, ?> --EXPECTF-- -TEST 0 +aes-128-ccm - TEST 0 +bool(true) +aes-128-ccm - TEST 1 +bool(true) +aes-256-ccm - TEST 0 bool(true) Warning: openssl_decrypt(): Setting of IV length for AEAD mode failed in %s on line %d diff --git a/ext/openssl/tests/openssl_encrypt_ccm.phpt b/ext/openssl/tests/openssl_encrypt_ccm.phpt index fb5dbbc849d..8c4c41f8187 100644 --- a/ext/openssl/tests/openssl_encrypt_ccm.phpt +++ b/ext/openssl/tests/openssl_encrypt_ccm.phpt @@ -10,15 +10,17 @@ if (!in_array('aes-256-ccm', openssl_get_cipher_methods())) --FILE-- $test) { - echo "TEST $idx\n"; - $ct = openssl_encrypt($test['pt'], $method, $test['key'], OPENSSL_RAW_DATA, - $test['iv'], $tag, $test['aad'], strlen($test['tag'])); - var_dump($test['ct'] === $ct); - var_dump($test['tag'] === $tag); +foreach ($methods as $method) { + $tests = openssl_get_cipher_tests($method); + foreach ($tests as $idx => $test) { + echo "$method - TEST $idx\n"; + $ct = openssl_encrypt($test['pt'], $method, $test['key'], OPENSSL_RAW_DATA, + $test['iv'], $tag, $test['aad'], strlen($test['tag'])); + var_dump($test['ct'] === $ct); + var_dump($test['tag'] === $tag); + } } // Empty IV error @@ -32,7 +34,13 @@ var_dump(strlen($tag)); var_dump(openssl_encrypt('data', $method, 'password', 0, str_repeat('x', 16), $tag, '', 1024)); ?> --EXPECTF-- -TEST 0 +aes-128-ccm - TEST 0 +bool(true) +bool(true) +aes-128-ccm - TEST 1 +bool(true) +bool(true) +aes-256-ccm - TEST 0 bool(true) bool(true) From 6559fe912661ca5ce5f0eeeb591d928451428ed0 Mon Sep 17 00:00:00 2001 From: Stanislav Malyshev Date: Sun, 20 Sep 2020 18:08:55 -0700 Subject: [PATCH 2/3] Do not decode cookie names anymore --- main/php_variables.c | 8 ++++++-- tests/basic/022.phpt | 10 +++++++--- tests/basic/023.phpt | 4 +++- tests/basic/bug79699.phpt | 22 ++++++++++++++++++++++ 4 files changed, 38 insertions(+), 6 deletions(-) create mode 100644 tests/basic/bug79699.phpt diff --git a/main/php_variables.c b/main/php_variables.c index 916fc1295b4..097c17d32af 100644 --- a/main/php_variables.c +++ b/main/php_variables.c @@ -492,7 +492,9 @@ SAPI_API SAPI_TREAT_DATA_FUNC(php_default_treat_data) size_t new_val_len; *val++ = '\0'; - php_url_decode(var, strlen(var)); + if (arg != PARSE_COOKIE) { + php_url_decode(var, strlen(var)); + } val_len = php_url_decode(val, strlen(val)); val = estrndup(val, val_len); if (sapi_module.input_filter(arg, var, &val, val_len, &new_val_len)) { @@ -503,7 +505,9 @@ SAPI_API SAPI_TREAT_DATA_FUNC(php_default_treat_data) size_t val_len; size_t new_val_len; - php_url_decode(var, strlen(var)); + if (arg != PARSE_COOKIE) { + php_url_decode(var, strlen(var)); + } val_len = 0; val = estrndup("", val_len); if (sapi_module.input_filter(arg, var, &val, val_len, &new_val_len)) { diff --git a/tests/basic/022.phpt b/tests/basic/022.phpt index 0ab70d4be76..bd1db137018 100644 --- a/tests/basic/022.phpt +++ b/tests/basic/022.phpt @@ -10,7 +10,7 @@ cookie1=val1 ; cookie2=val2%20; cookie3=val 3.; cookie 4= value 4 %3B; cookie1= var_dump($_COOKIE); ?> --EXPECT-- -array(10) { +array(12) { ["cookie1"]=> string(6) "val1 " ["cookie2"]=> @@ -19,11 +19,15 @@ array(10) { string(6) "val 3." ["cookie_4"]=> string(10) " value 4 ;" + ["%20cookie1"]=> + string(6) "ignore" + ["+cookie1"]=> + string(6) "ignore" ["cookie__5"]=> string(7) " value" - ["cookie_6"]=> + ["cookie%206"]=> string(3) "þæö" - ["cookie_7"]=> + ["cookie+7"]=> string(0) "" ["$cookie_8"]=> string(0) "" diff --git a/tests/basic/023.phpt b/tests/basic/023.phpt index ca5f1dcfbb1..0e2e0ac6694 100644 --- a/tests/basic/023.phpt +++ b/tests/basic/023.phpt @@ -10,9 +10,11 @@ c o o k i e=value; c o o k i e= v a l u e ;;c%20o+o k+i%20e=v;name="value","valu var_dump($_COOKIE); ?> --EXPECT-- -array(3) { +array(4) { ["c_o_o_k_i_e"]=> string(5) "value" + ["c%20o+o_k+i%20e"]=> + string(1) "v" ["name"]=> string(24) ""value","value",UEhQIQ==" ["UEhQIQ"]=> diff --git a/tests/basic/bug79699.phpt b/tests/basic/bug79699.phpt new file mode 100644 index 00000000000..fc3d3fedb08 --- /dev/null +++ b/tests/basic/bug79699.phpt @@ -0,0 +1,22 @@ +--TEST-- +Cookies Security Bug +--INI-- +max_input_vars=1000 +filter.default=unsafe_raw +--COOKIE-- +__%48ost-evil=evil; __Host-evil=good; %66oo=baz;foo=bar +--FILE-- + +--EXPECT-- +array(4) { + ["__%48ost-evil"]=> + string(4) "evil" + ["__Host-evil"]=> + string(4) "good" + ["%66oo"]=> + string(3) "baz" + ["foo"]=> + string(3) "bar" +} From 7bc112a142720fdbd553e5bcdbbe98c10d10928a Mon Sep 17 00:00:00 2001 From: Stanislav Malyshev Date: Mon, 28 Sep 2020 21:34:52 -0700 Subject: [PATCH 3/3] Update NEWS & UPGRADING --- NEWS | 7 +++++++ UPGRADING | 11 ++++++++--- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/NEWS b/NEWS index d69ca63874e..596c970821f 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,13 @@ PHP NEWS ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||| ?? ??? ????, PHP 7.2.34 +- Core: + . Fixed bug ##79699 (PHP parses encoded cookie names so malicious `__Host-` + cookies can be sent). (CVE-2020-7070) (Stas) + +- OpenSSL: + . Fixed bug ##79601 (Wrong ciphertext/tag in AES-CCM encryption for a 12 + bytes IV). (CVE-2020-7069) (Jakub Zelenka) 06 Aug 2020, PHP 7.2.33 diff --git a/UPGRADING b/UPGRADING index 52968a38575..c40fa8973e3 100644 --- a/UPGRADING +++ b/UPGRADING @@ -52,11 +52,16 @@ PHP 7.2 UPGRADE NOTES . The hash_hmac(), hash_hmac_file(), hash_pbkdf2() and hash_init() (with HASH_HMAC) functions no longer accept non-cryptographic hashes. -- JSON +- JSON: . The json_decode() option JSON_OBJECT_AS_ARRAY is used if the second parameter (assoc) is null. Previously JSON_OBJECT_AS_ARRAY was always ignored. +- SAPI: + . Starting with 7.2.34, incoming cookie names are not url-decoded. This was never + required by the standard, outgoing cookie names aren't encoded and this leads + to security issues (CVE-2020-7070). + - Session: . Removed register_globals related code and "!" can be used as $_SESSION key name. . Session is made to manage session status correctly and prevents invalid operations. @@ -69,7 +74,7 @@ PHP 7.2 UPGRADE NOTES session_unset(), session_write_close()/session_commit(), session_abort(), session_reset() . Functions prohibit invalid operations with regard to session status and - HTTP header status, returns correct bool return value. + HTTP header status, return correct bool return value. session_start(), session_set_cookie_params(), session_name(), session_module_name(), session_set_save_handler(), session_regenerate_id(), session_cache_limiter(), session_cache_expire(), session_unset(), session_destroy(), @@ -88,7 +93,7 @@ PHP 7.2 UPGRADE NOTES session_start() . When headers are already sent and try to set new INI values, session_name(), session_module_name(), session_save_path(), session_cache_limiter() and - session_cache_expire() are no longer works. Older PHPs accepts new values even + session_cache_expire() no longer work. Older PHPs accept new values even if new values will not be effective. This new corrected behavior may affect command line mode CLI scripts that manage sessions. Use output buffer just like web applications to resolve problems on