Out of bounds global memory read in BF_crypt triggered by password

Out of bounds global memory read in BF_crypt triggered by password_verify

Bug #72703	Out of bounds global memory read in BF_crypt triggered by password_verify
Submitted:	2016-07-29 01:40 UTC	Modified:	2016-08-29 17:09 UTC
From:	hanno at boeck dot de	Assigned:	ab (profile)
Status:	Closed	Package:	hash related
PHP Version:	7.0.9	OS:
Private report:	No	CVE-ID:	None

[2016-07-29 01:40 UTC] hanno at boeck dot de

Description:
------------
For certain invalid inputs the function password_verify() can trigger an out of bounds read in the function BF_crypt.

This was found with the help of american fuzzy lop and address sanitizer.

The error message from address sanitizer:
==32292==ERROR: AddressSanitizer: global-buffer-overflow on address 0x000002067660 at pc 0x000001272840 bp 0x7ffcae413070 sp 0x7ffcae413068
READ of size 1 at 0x000002067660 thread T0
    #0 0x127283f in BF_crypt /f/php/php-7.0.9/ext/standard/crypt_blowfish.c:775:3
    #1 0x12678be in php_crypt_blowfish_rn /f/php/php-7.0.9/ext/standard/crypt_blowfish.c:840:11
    #2 0x12f3421 in php_crypt /f/php/php-7.0.9/ext/standard/crypt.c:210:16
    #3 0x14a7589 in zif_password_verify /f/php/php-7.0.9/ext/standard/password.c:236:13
    #4 0x1a85c50 in ZEND_DO_ICALL_SPEC_HANDLER /f/php/php-7.0.9/Zend/zend_vm_execute.h:586:2
    #5 0x1901205 in execute_ex /f/php/php-7.0.9/Zend/zend_vm_execute.h:417:7
    #6 0x1902169 in zend_execute /f/php/php-7.0.9/Zend/zend_vm_execute.h:458:2
    #7 0x176117d in zend_execute_scripts /f/php/php-7.0.9/Zend/zend.c:1427:4
    #8 0x14f509b in php_execute_script /f/php/php-7.0.9/main/main.c:2494:14
    #9 0x1b475cc in do_cli /f/php/php-7.0.9/sapi/cli/php_cli.c:974:5
    #10 0x1b44485 in main /f/php/php-7.0.9/sapi/cli/php_cli.c:1344:18
    #11 0x7f2f1f41878f in __libc_start_main /var/tmp/portage/sys-libs/glibc-2.23-r2/work/glibc-2.23/csu/../csu/libc-start.c:289
    #12 0x462008 in _start (/mnt/ram/php/php+0x462008)

0x000002067660 is located 32 bytes to the left of global variable 'BF_atoi64' defined in '/f/php/php-7.0.9/ext/standard/crypt_blowfish.c:368:22' (0x2067680) of size 96
0x000002067660 is located 8 bytes to the right of global variable 'BF_magic_w' defined in '/f/php/php-7.0.9/ext/standard/crypt_blowfish.c:85:16' (0x2067640) of size 24
SUMMARY: AddressSanitizer: global-buffer-overflow /f/php/php-7.0.9/ext/standard/crypt_blowfish.c:775:3 in BF_crypt
Shadow bytes around the buggy address:
  0x000080404e70: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9
  0x000080404e80: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9
  0x000080404e90: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9
  0x000080404ea0: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9
  0x000080404eb0: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9
=>0x000080404ec0: f9 f9 f9 f9 f9 f9 f9 f9 00 00 00 f9[f9]f9 f9 f9
  0x000080404ed0: 00 00 00 00 00 00 00 00 00 00 00 00 f9 f9 f9 f9
  0x000080404ee0: 00 00 00 00 00 00 00 00 01 f9 f9 f9 f9 f9 f9 f9
  0x000080404ef0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x000080404f00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x000080404f10: 00 00 00 00 04 f9 f9 f9 f9 f9 f9 f9 00 f9 f9 f9
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Heap right redzone:      fb
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack partial redzone:   f4
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==32292==ABORTING


Test script:
---------------
<?php password_verify("","$2y$10$$");

Patches

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports

[2016-08-04 05:26 UTC] stas@php.net

Doesn't look like security issue, since second parameter of password_verify() should not be user-controlled by its nature.

[2016-08-04 07:20 UTC] stas@php.net

-Assigned To: +Assigned To: ab

[2016-08-14 23:42 UTC] ab@php.net

-Status: Assigned +Status: Feedback

[2016-08-14 23:42 UTC] ab@php.net

@hanno, could you please check whether the below makes ASAN happy? I'd also ask for the other invalid inputs you've mentioned, so they can be included in the test. Thanks. 

--- a/ext/standard/crypt.c
+++ b/ext/standard/crypt.c
@@ -204,6 +204,14 @@ PHPAPI zend_string *php_crypt(const char *password, const int pass_len, const ch
                                salt[1] == '2' &&
                                salt[3] == '$') {
                        char output[PHP_MAX_SALT_LEN + 1];
+                       int k = 7;
+
+                       while (isalnum(salt[k]) || '.' == salt[k] || '/' == salt[k]) {
+                               k++;
+                       }
+                       if (k != salt_len) {
+                               return NULL;
+                       }

[2016-08-28 04:22 UTC] php-bugs at lists dot php dot net

No feedback was provided. The bug is being suspended because
we assume that you are no longer experiencing the problem.
If this is not the case and you are able to provide the
information that was requested earlier, please do so and
change the status of the bug back to "Re-Opened". Thank you.

[2016-08-28 04:24 UTC] stas@php.net

-Status: No Feedback +Status: Open

[2016-08-29 15:01 UTC] ab@php.net

Thanks, Stas. I've lost this ticket after no feedback. As you say, a salt is produced by a backend, so this issue is barely triggerable from remote. I would tend therefore to include tihs patch in the upcomming RC to get some possible feedback, not waiting for the final.

Cheers.

[2016-08-29 17:09 UTC] stas@php.net

-Type: Security +Type: Bug

[2016-08-29 18:36 UTC] ab@php.net

-Status: Assigned +Status: Closed