#68986 pointer returned by php_stream_fopen_temporary_file not validated in memory.c #1051
Conversation
|
good catch, but since this problem also exists in 5.4, maybe you should make a PR against 5.4, then I can merge them upwards.. thanks :) |
|
there has been a change in internal function names between 5.4 and current master. i don't believe a 5.4 fix will be abled to be pushed up. in 5.4 the let me how how u want to handle it |
|
@devzer01 then okey, no problem, I can cherry-pick it... first of all, need try to make a test script... |
|
I will make a test case, no problem Sent via mobie
|
|
hi @laruence i added A test, please let me know, if i should test for something else too or if my approach to the test is inaccurate. |
There was a problem hiding this comment.
I doubt this will success, since you have chmod 444
There was a problem hiding this comment.
I set 444 to create a ready only temp dir, where earlier a segfault was
occurring, my test was to cover that segfault no longer occurs,
Strangely though when writing 1024 × 1024 × 2 bytes to a php://temp with
read only condition fwrite reports a slight lower number than 2mb has been
written
So maybe there is another place we need to fix
Sent via mobie
On Feb 5, 2015 2:58 PM, "Xinchen Hui" notifications@github.com wrote:
In tests/basic/bug68986.phpt
#1051 (comment):+--FILE--
+<?php
+if (substr(PHP_OS, 0, 3) == 'WIN') {
- die('skip.. only for unix');
+}
+$tempDir = getenv("TMPDIR");
+mkdir($tempDir . '/php68986');
+system("chmod 444 " . $tempDir . '/php68986');
+putenv("TMPDIR=" . $tempDir . '/php68986');
+
+$fp = fopen('php://temp', 'r+');
+$data = implode('', array_fill(0, (1024 * 1024 * 2), 'A'));
+var_dump(fwrite($fp, $data));
+fclose($fp);
+rmdir($tempDir . '/php68986');I doubt this will success, since you have chmod 444
—
Reply to this email directly or view it on GitHub
https://github.com/php/php-src/pull/1051/files#r24146247.
There was a problem hiding this comment.
maybe throwing an exception than returning a warning is a better approach? let me know.
|
at last, I think this is just a simple null pointer deference , and the test script is not easy to cleanup the temp dir, so I think just commit without test script. thanks |
|
thanks :) |
it seems that the pointer returned by php_stream_fopen_temporary_file / php_stream_fopen_tmpfile is not validated prior to use in memory.c in the case where a script is executed in an environment where the TMPDIR is not writable to the user, it will cause a SIGSEGV.
from the script side any code that utilize php://temp with 2MB+ buffer with the above condition will encounter this issue.
i have fixed the problem on all the calling code in memory.c using php_error_docref, i see that in phar they have handled the condition with zend_throw_exception_ex. i am not in a position to decide which one is best at this time. i think a friendly warning is better than a SIGSEGV at this point :)
i have validated this problem exists on github-master and releases 5.4.37, 5.4.17