Req #61435 [Com]: PHP-FPM logs are not readable by group/others by default
Edit report at https://bugs.php.net/bug.php?id=61435&edit=1 ID: 61435 Comment by: lekensteyn at gmail dot com Reported by:php at grange dot me Summary:PHP-FPM logs are not readable by group/others by default Status: Assigned Type: Feature/Change Request Package:FPM related PHP Version:5.3.10 Assigned To:fat Block user comment: N Private report: N New Comment: Patch refreshed for context in PHP 5.5.3 (no other changes). Could you please consider fixing this bug that has been present for over a year? A patch is available. Previous Comments: [2013-02-03 12:06:15] lekensteyn at gmail dot com I have attached a new patch (the old one was incompatible with 5.3). I have also changed "a+" to "a" as fpm_php_trace_dump is only writing to the file, not reading. According to the manpage, fdopen must have a mode that is compatible with the fd. In the old patch, there was a mismatch between a+ and O_WRONLY. [2012-03-19 11:29:10] php at grange dot me Description: Hello, errorlog, slowlog and accesslog are created with permissions set to 0600 by default on PHP 5.3 and 5.4. Those files are often owned by root (at least in our setup but probably in a lot of setups), which makes it not convenient for developers to read them. They may contain useful information, such as PHP crashes. Failing to fix it with umask in php-fpm init script (not mentioning the fact that it would affect php scripts too), I wrote a simple patch against PHP-5.3.10 to modify open() calls with 0644 perms. Note that Apache uses 0644 by default for its logs. Olivier -- Edit this bug report at https://bugs.php.net/bug.php?id=61435&edit=1
Req #61435 [Com]: PHP-FPM logs are not readable by group/others by default
Edit report at https://bugs.php.net/bug.php?id=61435&edit=1 ID: 61435 Comment by: lekensteyn at gmail dot com Reported by:php at grange dot me Summary:PHP-FPM logs are not readable by group/others by default Status: Assigned Type: Feature/Change Request Package:FPM related PHP Version:5.3.10 Assigned To:fat Block user comment: N Private report: N New Comment: No need to apologize, I do care but apparently it is not very high on the to-do list of PHP devs (if there are any). In January of this year, I reported an open_basedir-related security bug on this website, to which I still haven't got any reply yet. I wonder if somebody is actually using bug tracker besides users. Previous Comments: [2013-09-14 07:11:23] php at grange dot me Sorry, I fixed my patch on our systems but didn't take time to upload it here as nobody seemed to care. Thank you. [2013-09-07 13:53:38] lekensteyn at gmail dot com Patch refreshed for context in PHP 5.5.3 (no other changes). Could you please consider fixing this bug that has been present for over a year? A patch is available. [2013-02-03 12:06:15] lekensteyn at gmail dot com I have attached a new patch (the old one was incompatible with 5.3). I have also changed "a+" to "a" as fpm_php_trace_dump is only writing to the file, not reading. According to the manpage, fdopen must have a mode that is compatible with the fd. In the old patch, there was a mismatch between a+ and O_WRONLY. [2012-03-19 11:29:10] php at grange dot me Description: Hello, errorlog, slowlog and accesslog are created with permissions set to 0600 by default on PHP 5.3 and 5.4. Those files are often owned by root (at least in our setup but probably in a lot of setups), which makes it not convenient for developers to read them. They may contain useful information, such as PHP crashes. Failing to fix it with umask in php-fpm init script (not mentioning the fact that it would affect php scripts too), I wrote a simple patch against PHP-5.3.10 to modify open() calls with 0644 perms. Note that Apache uses 0644 by default for its logs. Olivier -- Edit this bug report at https://bugs.php.net/bug.php?id=61435&edit=1
Bug #53577 [Dup]: Regression (5.3.3-5.3.4) in open_basedir with a trailing forward slash
Edit report at http://bugs.php.net/bug.php?id=53577&edit=1 ID: 53577 User updated by:lekensteyn at gmail dot com Reported by:lekensteyn at gmail dot com Summary:Regression (5.3.3-5.3.4) in open_basedir with a trailing forward slash Status: Duplicate Type: Bug Package:Safe Mode/open_basedir Operating System: Windows 7 PHP Version:5.3.4 Block user comment: N Private report: N New Comment: This is related to bug #53352 , but not an exact duplicate. I've just added a patch on fopen_wrappers.c from the PHP 5.3 branch, r305698 ( http://svn.php.net/viewvc/php/php-src/branches/PHP_5_3/main/fopen_wrappers.c?view=markup&pathrev=305698 ) The patch fixed it for me. Previous Comments: [2010-12-20 07:34:40] ahar...@php.net Duplicate of bug #53352. [2010-12-19 23:58:32] lekensteyn at gmail dot com I'm just guessing, replacing the following: -- snip -- if (basedir[strlen(basedir) - 1] == PHP_DIR_SEPARATOR) { if (resolved_basedir[resolved_basedir_len - 1] != PHP_DIR_SEPARATOR) { resolved_basedir[resolved_basedir_len] = PHP_DIR_SEPARATOR; resolved_basedir[++resolved_basedir_len] = '\0'; } } else { resolved_basedir[resolved_basedir_len++] = PHP_DIR_SEPARATOR; resolved_basedir[resolved_basedir_len] = '\0'; } -- snip -- with -- snip -- if (basedir[strlen(basedir) - 1] == PHP_DIR_SEPARATOR) { if (resolved_basedir[resolved_basedir_len - 1] != PHP_DIR_SEPARATOR) { resolved_basedir[resolved_basedir_len] = PHP_DIR_SEPARATOR; resolved_basedir[++resolved_basedir_len] = '\0'; } #if defined(PHP_WIN32) || defined(NETWARE) } else if (basedir[strlen(basedir) - 1] != '/') { #else } else { #endif resolved_basedir[resolved_basedir_len++] = PHP_DIR_SEPARATOR; resolved_basedir[resolved_basedir_len] = '\0'; } -- snip -- should work. Under Windows, PHP_DIR_SEPARATOR is a backslash. So we check here if it is a forward slash. -------------------- [2010-12-19 23:44:46] lekensteyn at gmail dot com Description: Downloaded PHP 5.3.3 from: http://windows.php.net/downloads/releases/archives/php-5.3.3-nts-Win32-VC9-x86.zip Downloaded PHP 5.3.4 from: http://windows.php.net/downloads/releases/php-5.3.4-nts-Win32-VC9-x86.zip The following settings has the expected results in both PHP 5.3.3 and PHP 5.3.4 open_basedir="C:\twlan\" open_basedir="C:\twlan" open_basedir="C:/twlan" open_basedir="C:/twlan\" The following setting breaks open_basedir in PHP 5.3.4, but works fine in 5.3.3. open_basedir="C:/twlan/" So, the trailing forward slash on open_basedir makes every path invalid. Changes between 5.3.3 and 5.3.4: http://svn.php.net/viewvc/php/php-src/branches/PHP_5_3/main/fopen_wrappers.c?r1=301440&r2=306091 I think the bug was introduced in http://svn.php.net/viewvc/php/php-src/trunk/main/fopen_wrappers.c?r1=305098&r2=305698 --- begin code --- @@ -228,6 +234,9 @@ resolved_basedir[resolved_basedir_len] = PHP_DIR_SEPARATOR; resolved_basedir[++resolved_basedir_len] = '\0'; } + } else { + resolved_basedir[resolved_basedir_len++] = PHP_DIR_SEPARATOR; + resolved_basedir[resolved_basedir_len] = '\0'; } resolved_name_len = strlen(resolved_name); --- end code --- PHP_DIR_SEPARATOR is "\\" on Windows. Test script: --- Expected result: string(22) "C:\twlan\htdocs\combot" string(15) "C:\twlan\htdocs" string(8) "C:\twlan" Warning: realpath(): open_basedir restriction in effect. File(C:\) is not within the allowed path(s): (C:/twlan/) in C:\twlan\htdocs\combot\php-bug.php on line 7 bool(false) Actual result: -- Warning: realpath(): open_basedir restriction in effect. File(C:\twlan\htdocs) is not within the allowed path(s): (C:/twlan/) in C:\twlan\htdocs\combot\php-bug.php on line 5 bool(false) Warning: realpath(): open_basedir restriction in effect. File(C:\tw
Bug #53352 [Com]: open_basedir does not pass through files with matching path
Edit report at http://bugs.php.net/bug.php?id=53352&edit=1 ID: 53352 Comment by: lekensteyn at gmail dot com Reported by:dmitrij at stepanov dot lv Summary:open_basedir does not pass through files with matching path Status: Closed Type: Bug Package:Safe Mode/open_basedir Operating System: Windows 7 PHP Version:5.3SVN-2010-11-19 (SVN) Assigned To:pajoye Block user comment: N Private report: N New Comment: Please see bug #53577 (marked as dupe), the patch provided was incomplete. Direct link to the patch: http://bugs.php.net/patch-display.php?bug_id=53577&patch=open_basedir-trailing-slash-fix-PHP_5_3&revision=latest Previous Comments: [2010-12-09 18:04:39] paj...@php.net Automatic comment from SVN on behalf of pajoye Revision: http://svn.php.net/viewvc/?view=revision&revision=306136 Log: - missing merge fix for #53352 [2010-11-24 10:17:39] paj...@php.net This bug has been fixed in SVN. Snapshots of the sources are packaged every three hours; this change will be in the next snapshot. You can grab the snapshot at http://snaps.php.net/. Thank you for the report, and for helping us make PHP better. [2010-11-24 10:09:17] dmitrij at stepanov dot lv Sorry, my bad. Missed the "equal or" opcode :) r305698 works fine, issue is gone. Thanks. [2010-11-24 09:59:32] paj...@php.net Superior or equal to r305698, the r305698 is there :) [2010-11-24 07:24:08] dmitrij at stepanov dot lv Still see no snap at http://rmtools.php.net/snaps/ that is superior to r305698. Once it's there, I will reply with the results. The remainder of the comments for this report are too long. To view the rest of the comments, please view the bug report online at http://bugs.php.net/bug.php?id=53352 -- Edit this bug report at http://bugs.php.net/bug.php?id=53352&edit=1
Doc->Bug #54374 [Opn]: Insufficient validating of upload name leading to corrupted $_FILES indices
Edit report at https://bugs.php.net/bug.php?id=54374&edit=1 ID: 54374 User updated by:lekensteyn at gmail dot com Reported by:lekensteyn at gmail dot com Summary:Insufficient validating of upload name leading to corrupted $_FILES indices Status: Open -Type: Documentation Problem +Type: Bug Package:Variables related Operating System: All -PHP Version:5.3.6 +PHP Version:5.3.8 Block user comment: N Private report: N New Comment: This is not a documentation bug and still exists in PHP 5.3.8. Previous Comments: [2011-03-24 20:07:31] lekensteyn at gmail dot com Description: SAPI: Apache 2 module (it should apply to other SAPI's which accepts uploads as well) OS: Debian 6 (it should apply to other OSes as well) PHP: 5.3.6 (from source, test compile: ./configure --prefix=/tmp/diebug --disable-all --with-apxs2=/tmp/diebug/bin/apxs --disable-cli) Upload names with brackets ([ and ]) are created for creating arrays of files. Any array index or variable name containing a bracket should be invalid. The current implementation only checks whether more closing brackets are detected than opening brackets. Related files http://lxr.php.net/opengrok/xref/PHP_5_3/main/rfc1867.c#990 http://lxr.php.net/opengrok/xref/PHP_TRUNK/main/rfc1867.c#920 Relevant code: --snip-- /* New Rule: never repair potential malicious user input */ if (!skip_upload) { long c = 0; tmp = param; while (*tmp) { if (*tmp == '[') { c++; } else if (*tmp == ']') { c--; if (tmp[1] && tmp[1] != '[') { skip_upload = 1; break; } } if (c < 0) { skip_upload = 1; break; } tmp++; } } --snip-- So names like `test]` and `test[]]` are invalid, but names like `test[` pass this test. Now it gets worse, the upload is accepted and without checking the name, and registered: --snip-- if (is_arr_upload) { snprintf(lbuf, llen, "%s[name][%s]", abuf, array_index); } else { snprintf(lbuf, llen, "%s[name]", param); } if (s && s > filename) { register_http_post_files_variable(lbuf, s+1, http_post_files, 0 TSRMLS_CC); } else { register_http_post_files_variable(lbuf, filename, http_post_files, 0 TSRMLS_CC); } --snip-- register_http_post_files_variable calls safe_php_register_variable: --snip if (override_protection || !is_protected_variable(var TSRMLS_CC)) { php_register_variable_safe(var, strval, val_len, track_vars_array TSRMLS_CC); } --snip-- override_protection is false, the only condition that checks whether the variable name is accepted is the is_protected_variable call, passing the upload name. The variable name is normalized using normalize_protected_variable() and then checked for existence in the $_FILES array. The normalization function normalize_protected_variable checks whether a closing bracket is found, and otherwise uses the following string as index: --snip-- indexend = strchr(index, ']'); indexend = indexend ? indexend + 1 : index + strlen(index); --snip-- This implies that the index name can contain a opening bracket as well, which will be accepted and passed directly to php_register_variable_safe. The suggested patch adds a check to ensure that the leftover open brackets is always zero. If not, it simply drops the upload (better safe than sorry). Test script: --- %s', htmlspecialchars(print_r($_FILES, true))); } ?> Expected result: I expected to see "OK expected result" and an empty array dump because the name is invalid. Actual result: -- The test script produces "Unexpected result". The upload is accepted but the $_FILES array is corrupted: Array ( [test] => Array ( [[name] => [[type] => [[tmp_name] => [[error] => 4 [[size] => 0 ) ) -- Edit this bug report at https://bugs.php.net/bug.php?id=54374&edit=1
Req #61435 [Com]: PHP-FPM logs are not readable by group/others by default
Edit report at https://bugs.php.net/bug.php?id=61435&edit=1 ID: 61435 Comment by: lekensteyn at gmail dot com Reported by:php at grange dot me Summary:PHP-FPM logs are not readable by group/others by default Status: Assigned Type: Feature/Change Request Package:FPM related PHP Version:5.3.10 Assigned To:fat Block user comment: N Private report: N New Comment: I have attached a new patch (the old one was incompatible with 5.3). I have also changed "a+" to "a" as fpm_php_trace_dump is only writing to the file, not reading. According to the manpage, fdopen must have a mode that is compatible with the fd. In the old patch, there was a mismatch between a+ and O_WRONLY. Previous Comments: [2012-03-19 11:29:10] php at grange dot me Description: Hello, errorlog, slowlog and accesslog are created with permissions set to 0600 by default on PHP 5.3 and 5.4. Those files are often owned by root (at least in our setup but probably in a lot of setups), which makes it not convenient for developers to read them. They may contain useful information, such as PHP crashes. Failing to fix it with umask in php-fpm init script (not mentioning the fact that it would affect php scripts too), I wrote a simple patch against PHP-5.3.10 to modify open() calls with 0644 perms. Note that Apache uses 0644 by default for its logs. Olivier -- Edit this bug report at https://bugs.php.net/bug.php?id=61435&edit=1