php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login

go to bug id or search bugs for

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
View Developer Edit
 [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: Sat Dec 21 15:01:29 2024 UTC