Req #61435 [Com]: PHP-FPM logs are not readable by group/others by default

2013-09-07 Thread lekensteyn at gmail dot com
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

2013-09-14 Thread lekensteyn at gmail dot com
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

2010-12-20 Thread lekensteyn at gmail dot com
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

2010-12-20 Thread lekensteyn at gmail dot com
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

2011-11-18 Thread lekensteyn at gmail dot com
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

2013-02-03 Thread lekensteyn at gmail dot com
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