View Issue Details

IDProjectCategoryView StatusLast Update
0001407XdebugStacktracespublic2017-04-14 11:59
Reporterchelmertz Assigned Toderick  
PrioritynormalSeveritycrashReproducibilityalways
Status resolvedResolutionsuspended 
Summary0001407: Not checking for NULL causes segfault
Description

(Marking this bug as private, feel free to make it public if you think this is not a serious thing).

Hi, we have an in house developed PHP extension that I can probably not share publicly, so I will try to guide you through the problem as I see it.

Steps To Reproduce

We are throwing an exception of our own exception class (also implemented in PHP's C bindings) using zend_throw_exception_ex().

We end up in zend_exception_error()[1] which somehow does not recognize the file property of our exception (perhaps exceptions that we write ourselves typically do not include a file property? I would be interested to know).

As you can see in [1], Zend expects the error callback to accept NULL for the filename, but Xdebug's xdebug_error_cb() always assume that it is safe to strdup() the filename [2] and segfaults when it reaches this state. The output of GDB's "bt is here:

(gdb) run lala.php
Starting program: /usr/bin/php lala.php
warning: no loadable sections found in added symbol-file system-supplied DSO at 0x7ffff7ffa000
[Thread debugging using libthread_db enabled]

Program received signal SIGSEGV, Segmentation fault.
strlen_sse2 () at ../sysdeps/x86_64/strlen.S:32
32 movdqu (%rdi), %xmm1
Missing separate debuginfos, use: debuginfo-install libedit-2.11-4.20080712cvs.1.el6.x86_64 libidn-1.18-2.el6.x86_64 libtidy-0.99.0-19.20070615.1.el6.x86_64 lua-5.1.4-4.1.el6.x86_64
(gdb) bt
#0
strlen_sse2 () at ../sysdeps/x86_64/strlen.S:32
#1 0x00007ffff51f3016 in __strdup (s=0x0) at strdup.c:42
0000002 0x00007ffff408154b in xdebug_error_cb (type=1, error_filename=0x0, error_lineno=0, format=0x67888c "Uncaught %s\n thrown", args=0x7fffffffbd20) at /usr/src/debug/php-pecl-xdebug-2.1.4/xdebug-2.1.4/xdebug_stack.c:496
0000003 0x00000000005bf8d3 in zend_error_va (type=<value optimized out>, file=<value optimized out>, lineno=<value optimized out>, format=<value optimized out>) at /usr/src/debug/php-5.3.3/Zend/zend_exceptions.c:752
0000004 0x00000000005bfa68 in zend_exception_error (exception=0xc72f50, severity=1) at /usr/src/debug/php-5.3.3/Zend/zend_exceptions.c:792
0000005 0x00000000005a81a2 in zend_execute_scripts (type=8, retval=0x0, file_count=3) at /usr/src/debug/php-5.3.3/Zend/zend.c:1259
0000006 0x0000000000555fb8 in php_execute_script (primary_file=0x7fffffffe500) at /usr/src/debug/php-5.3.3/main/main.c:2268
0000007 0x00000000006324c5 in main (argc=2, argv=0x7fffffffe708) at /usr/src/debug/php-5.3.3/sapi/cli/php_cli.c:1192

1: https://github.com/php/php-src/blob/c6982995504b6e21e8a5ade29cfb16a55196dc43/Zend/zend_exceptions.c#L1014 (URL is for PHP's current master version, so this is still relevant)
2: https://github.com/xdebug/xdebug/blob/126514cf121ce3ecf6f92d26c1f661cb06e53406/xdebug_stack.c#L644 (URL is for Xdebug's current master version, so this is still relevant)

Additional Information

Old versions, I know :( They are/were receiving security patches from Red Hat at least, but I think the bug still exists in the current version of Xdebug.

php-pecl-xdebug-2.1.4-2.el6.x86_64
php-5.3.3-48.el6_8.x86_64
centos-release-6-8.el6.centos.12.3.x86_64

I am no C programmer so feel free to comment on what I did wrong or what else you would like to see. I am leaving this job in the beginning of April though, so feel free to contact me earlier than that if you need more information :)

TagsNo tags attached.
Operating SystemCentOS 6
PHP Version5.3.3

Activities

derick

2017-03-14 11:19

administrator   ~0004233

Can you please share your lala.php script - I need a reproducible case before I can verify and fix it (and add a test case).

chelmertz

2017-03-14 12:44

reporter   ~0004234

Hi Derick,

The lala.php script is really simple and is no good for a minimally reproducible test case sadly (it constructs an object and calls a method that throws an exception, because the constructor did not receive good enough arguments).

Sadly I am not able to share the full code since it is not open source. In order to create a minimal reproducible case, I would need a couple of hours and I don't have that the next few days. I will try to come back to this in the end of this week.

If you want to try something out yourself, you could try to create a PHP module that includes a zend_class_entry that is an exception, and throw that exception from within the PHP module with zend_throw_exception().

chelmertz

2017-03-17 14:58

reporter   ~0004235

Hi again Derick,

Too many other things needed getting done at work this week. I'll try to set some time off next week.

Have a nice weekend!

chelmertz

2017-04-04 10:53

reporter   ~0004250

All right, I didn't get any time allocated to do this and I don't have access to this triggering code any longer. I'll share this ticket ID with my team but I'm unsure if they are interested in creating a PoC.

Sadly, my best tip until then, is to read the code and try to follow the variable-that-can-be-NULL-but-is-unaccounted-for backwards to the call site.

derick

2017-04-14 11:59

administrator   ~0004262

I have had another look, and the assumption that the error_file is not NULL is not in just one place. As I can't really see how this can be NULL, I am going to mark this bug as suspended, until there is a proof of concept.

thanks for the report!
Derick

Issue History

Date Modified Username Field Change
2017-03-14 09:13 chelmertz New Issue
2017-03-14 11:19 derick Note Added: 0004233
2017-03-14 11:19 derick Assigned To => derick
2017-03-14 11:19 derick Status new => feedback
2017-03-14 11:19 derick View Status private => public
2017-03-14 12:44 chelmertz Note Added: 0004234
2017-03-14 12:44 chelmertz Status feedback => assigned
2017-03-17 14:58 chelmertz Note Added: 0004235
2017-04-04 10:53 chelmertz Note Added: 0004250
2017-04-14 11:59 derick Note Added: 0004262
2017-04-14 11:59 derick Status assigned => resolved
2017-04-14 11:59 derick Resolution open => suspended