php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Sec Bug #72262 int/size_t confusion in SplFileObject::fread
Submitted: 2016-05-25 09:44 UTC Modified: 2016-06-27 00:14 UTC
From: taoguangchen at icloud dot com Assigned: stas (profile)
Status: Closed Package: SPL related
PHP Version: 5.5.35 OS:
Private report: No CVE-ID: 2016-5770
Password:
Status:
Package:
Bug Type:
Summary:
From: taoguangchen at icloud dot com
New email:
PHP Version: OS:

 

 [2016-05-25 09:44 UTC] taoguangchen at icloud dot com
Description:
------------
int/size_t confusion in SplFileObject::fread

this bug similar with bug#72114

```
SPL_METHOD(SplFileObject, fread)
{
	spl_filesystem_object *intern = (spl_filesystem_object*)zend_object_store_get_object(getThis() TSRMLS_CC);
	long length = 0;

	if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "l", &length) == FAILURE) {
		return;
	}

	if (length <= 0) {
		php_error_docref(NULL TSRMLS_CC, E_WARNING, "Length parameter must be greater than 0");
		RETURN_FALSE;
	}

	Z_STRVAL_P(return_value) = emalloc(length + 1);
	Z_STRLEN_P(return_value) = php_stream_read(intern->u.file.stream, Z_STRVAL_P(return_value), length);

	/* needed because recv/read/gzread doesnt put a null at the end*/
	Z_STRVAL_P(return_value)[Z_STRLEN_P(return_value)] = 0;
	Z_TYPE_P(return_value) = IS_STRING;
}
```

PoC:
```
<?php

ini_set('memory_limit', -1);
$filename = '/dev/zero';
$file = new SplFileObject($filename, 'r');
$file->fread(2147483648);

?>
```

Fix:
```
		RETURN_FALSE;
	}
	
+	if (length > INT_MAX) {
+		php_error_docref(NULL TSRMLS_CC, E_WARNING, "Length parameter must be no more than %d", INT_MAX);
+		RETURN_FALSE;
+	}

	Z_STRVAL_P(return_value) = emalloc(length + 1);
```


Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2016-06-16 05:00 UTC] stas@php.net
-Assigned To: +Assigned To: stas
 [2016-06-16 05:00 UTC] stas@php.net
FIx in security repo as 7245bff300d3fa8bacbef7897ff080a6f1c23eba, also https://gist.github.com/8614d3d2afaf85b1fa6acb93a5bfff49
 [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=7245bff300d3fa8bacbef7897ff080a6f1c23eba
Log: Fix bug #72262 - do not 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=7245bff300d3fa8bacbef7897ff080a6f1c23eba
Log: Fix bug #72262 - do not 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=7245bff300d3fa8bacbef7897ff080a6f1c23eba
Log: Fix bug #72262 - do not 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=7245bff300d3fa8bacbef7897ff080a6f1c23eba
Log: Fix bug #72262 - do not 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=7245bff300d3fa8bacbef7897ff080a6f1c23eba
Log: Fix bug #72262 - do not overflow int
 [2016-06-23 12:50 UTC] kaplan@php.net
-CVE-ID: +CVE-ID: 2016-5770
 [2016-06-25 02:13 UTC] seth dot arnold at canonical dot com
What happens if length == INT_MAX? Won't that cause emalloc(length + 1) to fail?

Thanks
 [2016-06-27 00:14 UTC] stas@php.net
emalloc gets size_t, so it'll probably try to allocate a lot of memory - which will most probably fail.
 
PHP Copyright © 2001-2017 The PHP Group
All rights reserved.
Last updated: Sun Nov 19 01:31:42 2017 UTC