php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #74860 Uncaught exceptions not being formatted properly when error_log set to "syslog"
Submitted: 2017-07-05 20:36 UTC Modified: 2017-08-19 14:07 UTC
Votes:1
Avg. Score:4.0 ± 0.0
Reproduced:1 of 1 (100.0%)
Same Version:1 (100.0%)
Same OS:1 (100.0%)
From: philipp at redfish-solutions dot com Assigned: ab (profile)
Status: Closed Package: Output Control
PHP Version: 7.1.6 OS: linux 4.9.30
Private report: No CVE-ID: None
 [2017-07-05 20:36 UTC] philipp at redfish-solutions dot com
Description:
------------
I'm using php-7.1.6 on Linux (LEDE master).  During development, I had an uncaught exception which generated the diagnostic:

2017-07-05T11:56:20-06:00 PowercodeBMU Powercode: Error: Call to undefined function fetch() in /www/lib/php/LogController.php:48
Stack trace:
#0 /tmp/foo.php(7): LogIterator->valid()
#1 {main}

i.e. 4 lines of text with embedded newlines.  syslog() doesn't handle this correctly, as each line is supposed to be prefixed with a timestamp, hostname, and tag... but only the first line is formatted correctly because syslog() is handling the buffer as a single string.

The correct behavior is to explode this message on newlines and then write each part to syslog individually.

My php.ini file contained:

error_log = "syslog";


Test script:
---------------
Just about any uncaught exception should trigger this.

Expected result:
----------------
Correct logging should look like:

2017-07-05T11:56:20-06:00 PowercodeBMU Powercode: Error: Call to undefined function fetch() in /www/lib/php/LogController.php:48
2017-07-05T11:56:20-06:00 PowercodeBMU Powercode: Stack trace:
2017-07-05T11:56:20-06:00 PowercodeBMU Powercode: #0 /tmp/foo.php(7): LogIterator->valid()
2017-07-05T11:56:20-06:00 PowercodeBMU Powercode: #1 {main}

note that each line is prefixed identically.

Actual result:
--------------
Per description.

Patches

1006-multiline-syslog (last revision 2017-07-05 20:37 UTC by philipp at redfish-solutions dot com)

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2017-07-05 20:43 UTC] philipp at redfish-solutions dot com
Citing RFC 3164:

4.1.3 MSG Part of a syslog Packet

   The MSG part will fill the remainder of the syslog packet.  This will
   usually contain some additional information of the process that
   generated the message, and then the text of the message.  There is no
   ending delimiter to this part.  The MSG part of the syslog packet
   MUST contain visible (printing) characters. [...]
 [2017-07-05 22:12 UTC] hanskrentel at yahoo dot de
The MSG can not contain any non-printable characters (below %d32, higher than %d126).

currently those invalid MSG characters are not treated in any way, there is no input validation.

this patch tries to address the existing flaw.

but: this patch turns a single MSG into multiple messages while keeping invalid characters.

I don't think there is an overall conses on how to handle this, the RFC suggests that a receiver has to deal with that if non-printable (non-allowed) characters are used in the MSG part. I can imagine similar things when using UTF-8 in the MSGs containing octet sequences leaving the %d32-126 range.
 [2017-07-05 23:18 UTC] philipp at redfish-solutions dot com
> this patch tries to address the existing flaw

> but: this patch turns a single MSG into multiple messages while keeping invalid characters.

The patch is a very specific point-fix: it attempts to handle the known case of embedded newlines being generated by built-in code itself. Newlines are a specific issue and how to handle them in the case of syslog is tacitly understood: if syslog is a line-oriented logging protocol (which it definitely is), then it must be sent multiline messages as multiple single lines.

The larger, more abstract problem of how to handle *any* control character is not what is being addressed.

> I don't think there is an overall conses [sic] on how to handle this, the RFC suggests that a receiver has to deal with that if non-printable (non-allowed) characters are used in the MSG part.

That's entirely irrelevant: this bug is how the sender should properly format his messages, not how the receiver should handle malformed messages as you purport.

We most certainly do know how to handle things on the sending side: send well-formed messages.  There's no equivocation on this point.
 [2017-07-05 23:25 UTC] philipp at redfish-solutions dot com
For what it's worth, I'm using syslog-ng 3.9.1 on my system.

In the case of using rsyslog, the message would have been reformatted as it was written to disk with newlines being replaced literally with "\012" (i.e. an escaped octal sequence).  Also not desirable.
 [2017-07-05 23:26 UTC] spam2 at rhsoft dot net
> I don't think there is an overall conses on how to handle this, 
> the RFC suggests that a receiver has to deal with that if 
> non-printable (non-allowed) characters are used in the MSG part

yeah, that's how to create security issues as mod_rewrite did in case one did "cat logfile" leading to execute commands because of not properly filtered control chars
 [2017-07-11 04:16 UTC] philipp at redfish-solutions dot com
> yeah, that's how to create security issues as mod_rewrite did in case one did "cat logfile" leading to execute commands because of not properly filtered control chars

Sorry, not familiar with that CVE.  What are the details and how does it relate?
 [2017-07-26 16:35 UTC] philipp at redfish-solutions dot com
Can we please get movement on this?  There's a fix attached.
 [2017-08-04 11:54 UTC] krakjoe@php.net
The fix is in the wrong place. The fix should be to modify the php_syslog function to conform with expectations. I believe a more robust fix would be preferable ...
 [2017-08-04 12:19 UTC] spam2 at rhsoft dot net
and error_log() should also take care of non-printable characters because otherwise it's possible to trigger logfiles with control chars and that can lead in "cat logifle" unexpected executes code from untrusted input part of the logging
 [2017-08-04 18:54 UTC] philipp at redfish-solutions dot com
> and error_log() should also take care of non-printable characters because otherwise it's possible to trigger logfiles with control chars and that can lead in "cat logifle" unexpected executes code from untrusted input part of the logging

I'm not sure that's a real problem.  Why would anyone be executing log files?

And even if they did, it's not enough to have "magic contents" in a file to be a problem, they also have to be at the correct offset in the file... which would be extremely hard to guarantee for a log file, since you can't know in advance what's already been logged to that file.

This seems like a non-issue.
 [2017-08-04 21:41 UTC] spam2 at rhsoft dot net
you clearly did not understand what I was talking about: with control chars in a text file it is possible that a simple "cat filename" leads to execute code sequences in the shell and some time ago there was even a CVE for mod_security lacking proper escapeing leading to execute code by just display the log file
 [2017-08-04 21:49 UTC] philipp at redfish-solutions dot com
I'm looking at:

https://www.acunetix.com/vulnerabilities/web/apache-error-log-escape-sequence-injection-vulnerability

Where it says:

"This version of Apache is vulnerable to escape character sequences injection into error log.This problem may be exploited when a vulnerable terminal emulator is used."

emphasis on the last 5 words.  The vulnerability is the broken terminal emulator.

Likewise for:

http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2009-4487
 [2017-08-04 21:58 UTC] spam2 at rhsoft dot net
GDAMNED:it does not matter where the vulnerability is - make sure that f***g Logfiles don't contain control chars at all - THAT IS HOW SECURITY WORKS and not by a sloppy "there should not happen something bad" idiot attitude
 [2017-08-04 22:05 UTC] philipp at redfish-solutions dot com
> The fix is in the wrong place. The fix should be to modify the php_syslog function to conform with expectations. I believe a more robust fix would be preferable ...

You know that there is no actual function called php_syslog(), right?  And that it's a macro which either points to std_syslog() or else to syslog() directly.  And both of those are in external libraries.  So how would I fix that?
 [2017-08-05 00:02 UTC] philipp at redfish-solutions dot com
> THAT IS HOW SECURITY WORKS and not by a sloppy "there should not happen something bad" idiot attitude

It's not sloppy.  It's realistic.

I've literally fixed thousands of potential exploits in my lifetime.  At one point, I was doing it full-time.

There's a limited amount of bandwidth to secure systems, and you need to expend your energy and resources wisely.

There are an infinite number of files which could theoretically have control characters dumped into them via an equally infinite number of vectors.

They can't all be fixed.

What can be fixed is the singular vulnerability in Xterm.

You also can't put a bandaid over the symptom while calling the cause simultaneously fixed.

That's not security.  That self-delusion.
 [2017-08-05 03:59 UTC] spam2 at rhsoft dot net
> So how would I fix that?

by simply sanitize the arguments passed to external libraries

> There's a limited amount of bandwidth to secure systems, 
> and you need to expend your energy and resources wisely

yes, by< simply sanitize *everything* passed to whatever function if it can contain untrusted input and log output *clearly* falls in that scope
 [2017-08-07 03:57 UTC] philipp at redfish-solutions dot com
> yes, by simply sanitize *everything* passed to whatever function if it can contain untrusted input and log output *clearly* falls in that scope

You said to fix php_syslog().  I pointed out that this was a #define to syslog().

I *am* fixing the way we call syslog.

Perhaps look at the patch first before telling me how it falls short.
 [2017-08-07 05:01 UTC] spam2 at rhsoft dot net
what is your problem? i just pointed out that control chars has to be filtered at a central point so that this affects syslog(), error_log(), trigger_error() and what not which leads in producing logfiles - not more and not less - dunno why you needed to make dumb answers like "why would anyone execute logfiles" and argue around at all

currently it just sucks that you need to filter logoutput in userland and in case of trigger_error have no way to don't break the html page by not use htmlentities() while at the same time they appear in the logfiles where nobody beeds them 

so what is my point? that currently the whole error handling / logging is a complete mess
 [2017-08-07 23:57 UTC] philipp at redfish-solutions dot com
> what is your problem?

On 7/11/2017 I asked what the vulnerability was, i.e. CVE number, etc. and you never responded.

I've been very indulgent about a vulnerability which you still have to substantiate.
 [2017-08-08 08:57 UTC] spam2 at rhsoft dot net
you even quoted at your own "This version of Apache is vulnerable to escape character sequences injection into error log.This problem may be exploited when a vulnerable terminal emulator is used" in the meantime and NO it is NOT worth to dicusss where and if and when a vulnerable terminal emulator may be used to view some logfile 

best practice is to avoid this whole discussion and just sanitize in one and for all at a central codepoint which is then used for everything dealing with write to logs and frankly it's likely done in a shorter time then discuss about it
 [2017-08-15 20:50 UTC] philipp at redfish-solutions dot com
This is two separate but vaguely related issues, but they should not be conflated because the considerations are very different.

Opening a separate bug report for the issue of how best to handle non NVT-ASCII, as bz #75077.
 [2017-08-16 18:57 UTC] philipp at redfish-solutions dot com
> best practice is to avoid this whole discussion and just sanitize in one and for all at a central codepoint which is then used for everything dealing with write to logs and frankly it's likely done in a shorter time then discuss about it

The "central codepoint" would be the logger, in that case, which is what writes to the file.

Not PHP, which is one of thousands of logging clients.

So you're effectively arguing for NOT fixing it in PHP if I've understood your point.
 [2017-08-19 14:07 UTC] ab@php.net
-Status: Open +Status: Closed -Assigned To: +Assigned To: ab
 [2017-08-19 14:07 UTC] ab@php.net
PR https://github.com/php/php-src/pull/2674 was merged.

Thanks.
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Sun Oct 06 01:01:27 2024 UTC