Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

preg_replace times out after upgrading to 8.2 when browscap setting is set #12885

Closed
ymTm7KuhCnIjkZAl2x2m2 opened this issue Dec 6, 2023 · 19 comments

Comments

@ymTm7KuhCnIjkZAl2x2m2
Copy link

Description

I had this issue after upgrading to 8.2 and I've created this example that shows the issue. My php.ini is exactly the production version with one change: browscap points to the latest lite_php_browscap.ini from browscap.org. I'm using IIS on Windows Server 2022.

The following code:

<?php

$string = 'The quick brown fox jumps over the lazy dog.';
$patterns = array();
$patterns[0] = '/quick/';
$patterns[1] = '/brown/';
$patterns[2] = '/fox/';
$replacements = array();
$replacements[0] = 'slow';
$replacements[1] = 'black';
$replacements[2] = 'bear';

echo preg_replace($patterns, $replacements, $string);

Resulted in this output:

HTTP Error 500.0 - Internal Server Error

But I expected this output instead:

The slow bear jumps over the lazy dog.

In fact, I do get this output if I comment out the browscap setting in php.ini. I also get this result if I remove many lines from the browscap file. I did not see this issue in PHP 8.1.

PHP Version

8.2.13

Operating System

Windows Server 2022

@nielsdos
Copy link
Member

nielsdos commented Dec 6, 2023

Do you know if it crashes immediately, or on request shutdown?
It is possible you're running into #12621 (which will be fixed in 8.2.14) or something similar.
There was another fix for browscap in 8.2.5, if that broke anything then testing on 8.2.4 should make it not crash. If 8.2.4 also crashes then it's probably the above-linked bug or a new one.
One annoying thing is that crashes aren't always reliable so you might need to try to load the page a couple of times.

@ymTm7KuhCnIjkZAl2x2m2
Copy link
Author

ymTm7KuhCnIjkZAl2x2m2 commented Dec 7, 2023

The browser shows it returning a 500 after 2.07 seconds. I'll give it a try on 8.2.4 tomorrow.

@ymTm7KuhCnIjkZAl2x2m2
Copy link
Author

So I'm actually not able to reproduce it with any of the pre-built Windows packages. I can only reproduce it with a custom one that we build. Here is the Configure Command:

cscript /nologo /e:jscript configure.js "--disable-all" "--enable-snapshot-build" "--enable-mbstring" "--enable-cli" "--enable-cgi" "--with-libxml" "--with-dom" "--enable-session" "--with-iconv" "--with-curl=d:\temp\php\phpdev\vs16\x64\deps\" "--enable-mbregex" "--enable-soap" "--enable-ctype" "--with-openssl=d:\temp\php\phpdev\vs16\x64\deps\" "--enable-bcmath" "--enable-filter" "--enable-zlib" "--with-mhash" "--with-pcre-jit" "--with-xml" "--enable-tokenizer"

@simonberger
Copy link

To me it is unlikely related to the previous browscap issues or browscap at all as those took just effect when a call to get_browser was actually done which is not the case here. At least not in the given example.

@nielsdos
Copy link
Member

nielsdos commented Dec 8, 2023

To me it is unlikely related to the previous browscap issues or browscap at all as those took just effect when a call to get_browser was actually done which is not the case here. At least not in the given example.

It's a little bit more subtle. In your case the actual corruption happened during the parsing of the browscap file, but only became exposed when get_browser was actually called. It's however possible, depending on the heap layout, that it triggers even without a call to get_browser.

Now, it could be possible of course that the browscap symptom in this issue is a red herring.

@ymTm7KuhCnIjkZAl2x2m2 I tried to reproduce this on my Windows VM but I couldn't. Are you able to checkout the latest source of branch PHP-8.2 and build the latest version and test?

@ymTm7KuhCnIjkZAl2x2m2
Copy link
Author

Sorry I was unable to build the PHP-8.2 branch.

@ymTm7KuhCnIjkZAl2x2m2
Copy link
Author

Okay I was able to build both the PHP-8.2 branch (as of 3 days ago) and version 8.2.4 and they both have the same issue.

@nielsdos
Copy link
Member

Ok no regresion then from that change.
Does it reproduce on the 8.2 development branch? I'm guessing it's fixed by the patch queued up for 8.2.14.

@nielsdos
Copy link
Member

And ofc when I press send I realise I misread your message... so it's not fixed and not a regression... bummer...
I'll try reproducing the issue again next week, can you share your phpinfo?

@ymTm7KuhCnIjkZAl2x2m2
Copy link
Author

Had to obfuscate some things and change html to txt. Also I built a version of 8.2.0 and the script worked on the first try and then not on subsequent attempts.
phpinfo.txt

@ymTm7KuhCnIjkZAl2x2m2
Copy link
Author

I'm able to reproduce this issue with a single line and browscap not enabled.

$test = strtolower(preg_replace(['/([A-Z]+)([A-Z][a-z])/', '/([a-z\d])([A-Z])/'], ['\\1_\\2', '\\1_\\2'], str_replace('_', '.', 'Framework')));

Event viewer shows:

Faulting application name: php-cgi.exe, version: 8.2.13.0, time stamp: 0x65a6eccb
Faulting module name: unknown, version: 0.0.0.0, time stamp: 0x00000000
Exception code: 0xc0000005
Fault offset: 0x00007ffb168f038b
Faulting process id: 0x22f8
Faulting application start time: 0x01da749308230ac0
Faulting application path: C:\Program Files (x86)\Company\App\php\php-cgi.exe
Faulting module path: unknown
Report Id: bddbee1c-6dff-42be-a075-256c9d6e65dd
Faulting package full name:
Faulting package-relative application ID:

@ymTm7KuhCnIjkZAl2x2m2
Copy link
Author

If I change pcre.recursion_limit to something smaller, it crashes half as often!

@ymTm7KuhCnIjkZAl2x2m2
Copy link
Author

Okay so here is an entire code block that I'm using to reproduce a regex crash. Without the $_SESSION code it doesn't seem to crash. With the code it crashes half of the time.

session_start();
$arr = array();
for($i=0;$i<1000000;$i++){
    $arr[$i] = 'abcdefghijklmnopqrstuvwxyz';
}
$_SESSION["foo"]=$arr;
$test = strtolower(preg_replace(['/([A-Z]+)([A-Z][a-z])/', '/([a-z\d])([A-Z])/'], ['\\1_\\2', '\\1_\\2'], str_replace('_', '.', 'Framework')));
die($test);

@ymTm7KuhCnIjkZAl2x2m2
Copy link
Author

I've upgraded to the latest 8.2, I've upgraded to 8.3.3, I've removed --with-pcre-jit and --enable-mbregex and it still crashes. Preg_replace is just completely unusable.

@nielsdos
Copy link
Member

I can't reproduce this on Linux. I'm going to try a custom Windows build again like you did and check with ASAN.

@nielsdos
Copy link
Member

I can't reproduce it on Windows either and ASAN shows nothing.
Does it only crash on CGI or on CLI too?

Try to compile a debug build, but change the following line in configure.js from:
ADD_FLAG('CFLAGS', ' /wd4996 ');
to
ADD_FLAG('CFLAGS', ' /wd4996 /fsanitize=address ');

Check what the output is on crash. If you get nothing special, try setting the USE_ZEND_ALLOC environment variable to 0.

@ymTm7KuhCnIjkZAl2x2m2
Copy link
Author

Okay! I think I've figured it out:

When we were on 8.1.X, we couldn't use PCRE 10.39 because of vulnerabilities. Somebody noticed that 8.2.X had 10.40 so we updated our version and made two code changes to get 8.1 to run with 10.40. However, they took it from the master branch instead of any 8.2 branch. I see that 8.3 has been updated to PCRE 10.42. So I updated to that (8.3.3 branch) and everything works.

So the real issue is that the master branch of PCRE is (or was?) in a weird state. For example, in php-src/ext/pcre/php_pcre.c, line 37 in the 8.3.3 branch is

#define PREG_REPLACE_EVAL			(1<<0)

While in master that line is absent. It seems like the master we used was randomly missing some 14 year old commits yet contained some from last year.

Feel free to close the ticket, or use it to get master in line with whatever it should be.

@nielsdos
Copy link
Member

Glad that your issue is resolved!

The master branch has PCRE2 10.43, and some other additions. So I can imagine there are some differences that could cause issues indeed.

or use it to get master in line with whatever it should be.

I just checked and don't find anything odd.

We also normally backport any critical bugs or vulnerability fixes from more recent pcre2 versions to lower stable versions of PHP that ship an older pcre2.

@nielsdos nielsdos closed this as not planned Won't fix, can't repro, duplicate, stale Mar 14, 2024
@MaxShirinzad

This comment was marked as off-topic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants