Skip to content

Conversation

@skvoboo
Copy link
Contributor

@skvoboo skvoboo commented Apr 14, 2017

The pointer returned by 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 parent directory of system TMP directory is not readable to the user, it will cause a application crash with Access Violation.

The issue is similar to the https://bugs.php.net/bug.php?id=68986

Steps to reproduce:

  1. Create C:\TEMP folder
  2. Create system user
  3. Add Deny Full control access rights for created user on C:\
  4. Remove Deny Full control access rights for created user on C:\TEMP
  5. Add Allow Full control access rights for created user on C:\TEMP
  6. Set TEMP and TMP system environment variables value to C:\TEMP
  7. Execute script by created user:
<?php
$fp = fopen('php://temp', 'w+b');
$ch = curl_init();
curl_setopt($ch, CURLOPT_FILE, $fp);


file = php_stream_fopen_tmpfile();
if (file == NULL) {
php_error_docref(NULL, E_WARNING, "Unable to create temporary file. Check permissions in temporary files directory.");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can fail to various reasons, so maybe could simplify the msg or even omit it if php_stream_fopen_tmpfile() already throws a warning.

Thanks.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can fail to various reasons, so maybe could simplify the msg or even omit it if php_stream_fopen_tmpfile() already throws a warning.

Message was simplified.

@krakjoe
Copy link
Member

krakjoe commented Apr 17, 2017

@weltling what's the status here ?

@weltling
Copy link
Contributor

@krakjoe oh, i didn't realize it were assigned. LGTM, can take care before RC, if not merged earlier.

Thanks.

@weltling
Copy link
Contributor

Merged as 793a8bd.

Thanks.

@weltling weltling closed this Apr 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants