XF 1.3 Highly-nested bbcode post causing php to crash

[Xon]

Hi,

I've got a large post(~100kb text) with highly-nested bbcode which is making php-workers near crash. It's actually a horribly encoded post by a user with at least ~17 layers of nesting, which I've instructed them how to avoid.

But I would prefer it couldn't crash the php-fpm worker processes with what appears to be a stack overflow.

I'm using php 5.5 from the IUS repos on CentoOS 6.5
"php -v" output:

PHP 5.5.14 (cli) (built: Jun 27 2014 12:26:01)
Copyright (c) 1997-2014 The PHP Group
Zend Engine v2.5.0, Copyright (c) 1998-2014 Zend Technologies
  with Zend OPcache v7.0.4-dev, Copyright (c) 1999-2014, by Zend Technologies

On my forum it can be posted, and on a local dev instance (Linux+nginx+php-fpm+mariadb stack, same versions of software as live) I can edit/reply to it with TinyMCE off (but not on!). I had some users report they could reply to it on the actual forums with the addon being active but it failed for me.

Originally I thought it was a problem with a the TinyMCE Quattro addon, but with help from @cclaerhout (here) it looks like an issue with stock Xenforo as well.

Is there any way to narrow down why this is occurring? The only error I can see from the php-fpm logs are;

[27-Jul-2014 08:15:39] WARNING: [pool www] child 23811 exited on signal 11 (SIGSEGV) after 5458.336405 seconds from start
[27-Jul-2014 08:15:39] NOTICE: [pool www] child 32674 started

I've got the post on hand, but haven't had a chance to reduce it down to just the bbcode to see exactly what limits I'm running into.

Editing the DB to remove 17 layers of nested [font=trebuchet ms] around the bulk of the post stopped the issue. The [center] tag appears to reset the font, so the user was just adding a new open font tag after a center, and kept doing that for 17 chunks.

Using xdebug, I can verify that recursion from bbcode parsing is only hitting ~147 functions deep. Which isn't that bad.

 

[cclaerhout]

I took a look at your message code and made it simple to reproduce the problem. The issue on my local board is only with the wysiwyg parser. It is caused because of how was used the font Bb Code. Here's a simpler code of what triggers the error:

[font=trebuchet ms]a17
[font=trebuchet ms]a16
[font=trebuchet ms]a15
[font=trebuchet ms]a14
[font=trebuchet ms]a13
[font=trebuchet ms]a12
[font=trebuchet ms]a11
[font=trebuchet ms]a10
[font=trebuchet ms]a9
[font=trebuchet ms]a8
[font=trebuchet ms]a7
[font=trebuchet ms]a6
[font=trebuchet ms]a5
[font=trebuchet ms]a4
[font=trebuchet ms]a3
[font=trebuchet ms]a2
[font=courier new]a1
[/font][/font][/font][/font][/font][/font][/font][/font][/font][/font][/font][/font][/font][/font][/font][/font][/font]

Edit: This code can be edited on XenForo ; I've tried again on my local board without any addon activated, it doesn't work.
Edit 2: If I delete the "filterFinalOutput" of the wysiwyg formater (then use the one from the base), the message can be edited.
Edit 3: the problem seems to come from this line:

preg_replace('#<break />\s*((' . $this->_blockTags . ')[^>]*>)?#i', "$close\\0$open", $inner);

 

[cclaerhout]

To check: I think there's a mistake in this regex:

        '#<(xf' . $i . ':((?>[a-z0-9-]+?)))([^>]*)(?<!/)>((?:(?>[^<]*?)|<)*)</\\1>#siU',

The first captured group doesn't seem correct. Try this instead:

        '#<(xf' . $i . ':((?>[a-z0-9-]+)))([^>]*)(?<!/)>((?:(?>[^<]*?)|<)*)</\\1>#siU',

Edit: even with that it still doesn't work with the original message of Xon.

 

[cclaerhout]

@Xon
I've managed to get your post content edited on my local board. On your test board (avoid to make such tests on your live board) :

1. change the regex as described above
2. increase the ThreadStackSize setting in your Apache configuration file "httpd.conf" (ref); for me on Windows:

   <IfModule mpm_winnt_module>
   ThreadStackSize 8388608
   </IfModule>

 

[Xon]

@cclaerhout Thanks for the help diagnosing exactly what is happening.

I had to set limits via /etc/security/limits.d/to tag the particular user that php-fpm is run by (it doesn't explicitly support setting the stacksize like apache, and I would prefer not to hack up the init.d script).

Hopefully I got the syntax right and put the changes in the right places.

Sadly, this isn't a real-fix besides rewriting how that regex is used.