PHP :: Bug #36928 :: error_log with invalid arguments crashes PHP

[2006-03-31 03:08 UTC] michaelw at webcentral dot com dot au

Description:
------------
This was noticed by a developer making a typo ( , instead of . ) when attempting to concat strings within the parameters of error_log. It is reported as a bug because it causes a segfault in PHP which causes the webserver to crash.

Reproduce code:
---------------
<html>
<body>
<?php
error_log("commas can crash ",($_SERVER['HTTPS'] != 'off'));
?>
<p>
Test..</p>
</body>
</html>


Expected result:
----------------
Presumably an error indicating that the 2nd parameter passed to error_log is invalid.

Actual result:
--------------
Program received signal SIGSEGV, Segmentation fault.


(gdb) bt
#0  0xfedb451c in strlen () from /usr/lib/libc.so.1
#1  0xfee06f88 in _doprnt () from /usr/lib/libc.so.1
#2  0xfee089e4 in fprintf () from /usr/lib/libc.so.1
#3  0x000d0970 in php_mail (to=0x0, subject=0x193868 "To: %s\n", message=0x0, headers=0x0, extra_cmd=0x0,
    tsrm_ls=0x0) at /opt/admin/build/php-4.4.2/ext/standard/mail.c:228


Presumably the variable should be sanity checked both in php_mail and the error_log function..

[2006-03-31 03:50 UTC] michaelw at webcentral dot com dot au

Hey,

This is a better 'Reproduce Code' (it doesn't attempt to send an email if the 2nd variable is a 0, and hence doesn't crash, so depending on what $_SERVER['HTTPS'] evaluated to for you, it might not have errored..):

<html>
<body>
<?php
error_log("commas can crash ",1);
?>
<p>
Test..</p>
</body>
</html>

I'm currently compiling the suggested CVS snapshot and will let you know when I have a result.

[2006-03-31 03:56 UTC] michaelw at webcentral dot com dot au

I've verified I can reproduce it with the latest CVS snapshot with a compile string of: 

./configure  --prefix=/opt/php --with-nsapi=/opt/sunapps/web --enable-debug

Its probably also worth noting I can replicate it using the CLI..

# gdb sapi/cli/php
GNU gdb 6.0
Copyright 2003 Free Software Foundation, Inc.
GDB is free software, covered by the GNU General Public License, and you are
welcome to change it and/or distribute copies of it under certain conditions.
Type "show copying" to see the conditions.
There is absolutely no warranty for GDB.  Type "show warranty" for details.
This GDB was configured as "sparc-sun-solaris2.9"...set
(gdb) set args /webdocs/school-hosting/centraloffice/mis-webcentral/www/crash2.php
(gdb) run
Starting program: /opt/admin/build/php4-STABLE-200603310035/sapi/cli/php /webdocs/school-hosting/centraloffice/mis-webcentral/www/crash2.php

Program received signal SIGSEGV, Segmentation fault.
0xff13451c in strlen () from /usr/lib/libc.so.1
(gdb) bt
#0  0xff13451c in strlen () from /usr/lib/libc.so.1
#1  0xff186f88 in _doprnt () from /usr/lib/libc.so.1
#2  0xff1889e4 in fprintf () from /usr/lib/libc.so.1
#3  0x0009c374 in php_mail (to=0x0, subject=0x15c8e0 "To: %s\n", message=0x0, headers=0xff1bc000 "",
    extra_cmd=0x2134a8 "s\022??", tsrm_ls=0x1a6278)
    at /opt/admin/build/php4-STABLE-200603310035/ext/standard/mail.c:228

[2006-03-31 06:33 UTC] arnar at 8 dot is

This crash is cosed by solaris's libc not checking the fprintf arguments, and php fault for passing in a NULL argumnet.

Link to patch: http://php.is/patch/mail.patch

Index: ext/standard/mail.c
===================================================================
RCS file: /repository/php-src/ext/standard/mail.c,v
retrieving revision 1.66.2.12.4.2
diff -u -r1.66.2.12.4.2 mail.c
--- ext/standard/mail.c	1 Jan 2006 13:46:57 -0000	1.66.2.12.4.2
+++ ext/standard/mail.c	31 Mar 2006 04:29:29 -0000
@@ -196,6 +196,10 @@
 		return 0;
 #endif
 	}
+	if (to == NULL && headers == NULL) {
+		php_error_docref(NULL TSRMLS_CC, E_WARNING, "Invalid arguments");
+		return 0;
+	}
 	if (extra_cmd != NULL) {
 		sendmail_cmd = emalloc (strlen (sendmail_path) + strlen (extra_cmd) + 2);
 		strcpy (sendmail_cmd, sendmail_path);
@@ -225,7 +229,9 @@
 			return 0;
 		}
 #endif
-		fprintf(sendmail, "To: %s\n", to);
+		if (to != NULL) {
+			fprintf(sendmail, "To: %s\n", to);
+		}
 		fprintf(sendmail, "Subject: %s\n", subject);
 		if (headers != NULL) {
 			fprintf(sendmail, "%s\n", headers);

[2006-03-31 06:40 UTC] bjori@php.net

And a patch for error_log() to require 'destination' when set to send email: http://php.is/bugs/36928/error_log.patch.txt

[2006-03-31 08:01 UTC] michaelw at webcentral dot com dot au

Thank you, this corrects the issue. 

Just one more thing for completeness..  in the mail.c patch you test the value of to before passing it to fprintf. Should the same thing be done for the value of subject ?

[2006-04-11 17:15 UTC] sniper@php.net

Please try using this CVS snapshot:

  http://snaps.php.net/php5.1-latest.tar.gz
 
For Windows:
 
  http://snaps.php.net/win32/php5.1-win32-latest.zip

[2006-04-19 01:00 UTC] php-bugs at lists dot php dot net

No feedback was provided for this bug for over a week, so it is
being suspended automatically. If you are able to provide the
information that was originally requested, please do so and change
the status of the bug back to "Open".

Patches

Pull Requests

History

	php.net \| support \| documentation \| report a bug \| advanced search \| search howto \| statistics \| random bug \| login
go to bug id or search bugs for


Copyright © 2001-2024 The PHP Group All rights reserved.	Last updated: Wed Apr 24 14:01:30 2024 UTC