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_split(): Compilation failed: escape sequence is invalid #92

Closed
erlangsec opened this issue Sep 11, 2019 · 7 comments
Closed

preg_split(): Compilation failed: escape sequence is invalid #92

erlangsec opened this issue Sep 11, 2019 · 7 comments
Assignees

Comments

@erlangsec
Copy link
Contributor

erlangsec commented Sep 11, 2019

I get the error message above, which looks like a regex compliation bug related to PHP 7.3, originating from:
/vendor/zbateson/mail-mime-parser/src/Header/Consumer/AbstractConsumer.php:187

According to this resource, e.g. "the hyphen now needs to be escaped" (if this could be a relevant case).

See also PHP 7.3 migration guide for PCRE:
https://wiki.php.net/rfc/pcre2-migration#backward_incompatible_changes

@zbateson
Copy link
Owner

Hi @erlangsec --

Could you give more details on where that may be happening, or even more helpful would be a test case? Currently it seems all tests are passing in 7.3 for version 1.1.5... but maybe I'm missing a test somewhere. I had a quick look around at the generated patterns as well but couldn't find any hyphens.

@erlangsec
Copy link
Contributor Author

Thanks for asking, the bug is triggered by any Received header, as far as I can tell, including the one from your original tests:

Received: from LeComputer (blah.host) by MyComputer ([1.2.2.2]) with ESMTP (TLS BLAH) ID 123; Wed, 17 May 2000 19:08:29 -0400

Apparently, this header is also treated with some special parsing patterns (which makes sense).

@erlangsec
Copy link
Contributor Author

I've found that the character \A does not compile, in /src/Header/Consumer/Received/GenericReceivedConsumer.php line 126.

The error is resolved by switching from PHP7.3 to PHP7.2. Or by removing/replacing the character.

@erlangsec
Copy link
Contributor Author

While I do not have any official source for this theory, it looks like since \A cannot be repeated, it cannot be contained in a group which characters can be repeated, and this is validated more strictly in PCRE2/PHP7.3.

Proposed fix in pull request #93.

@zbateson
Copy link
Owner

Excellent, thanks for that @erlangsec

I'll review your change and aim to release a fix this weekend... can't for the life of me think of why I grouped \A like that, so will have a look around.

Can you tell me if, without that fix, a unit test is failing for you and which one, and if your fix fixes it for you as well? It seems Travis on PHP 7.3 isn't erroring out, and I'm wondering if you may know why:

https://travis-ci.org/zbateson/mail-mime-parser/jobs/573601308

Thanks again!

@erlangsec
Copy link
Contributor Author

erlangsec commented Sep 21, 2019

Without proposed fix:
............................................................... 63 / 452 ( 13%) .......EEEEEE.EEEEEEEEEE..................EE..E................ 126 / 452 ( 27%) ...................................................EEEEE....... 189 / 452 ( 41%) ............................................................... 252 / 452 ( 55%) ............................................................... 315 / 452 ( 69%) ............................................................... 378 / 452 ( 83%) ............................................................... 441 / 452 ( 97%) ........... 452 / 452 (100%)

There were 24 errors:

1) ZBateson\MailMimeParser\Header\Consumer\Received\DomainConsumerTest::testConsumeParts preg_split(): Compilation failed: escape sequence is invalid in character class at offset 49
2) ZBateson\MailMimeParser\Header\Consumer\Received\GenericReceivedConsumerTest::testConsumeTokens preg_split(): Compilation failed: escape sequence is invalid in character class at offset 49

and so on ...

But with fix, no errors:
............................................................... 63 / 452 ( 13%) ............................................................... 126 / 452 ( 27%) ............................................................... 189 / 452 ( 41%) ............................................................... 252 / 452 ( 55%) ............................................................... 315 / 452 ( 69%) ............................................................... 378 / 452 ( 83%) ............................................................... 441 / 452 ( 97%) ........... 452 / 452 (100%)

My environment:
PHP 7.3.8-1+ubuntu18.04.1+deb.sury.org+1 (cli) (built: Aug 7 2019 09:52:12) ( NTS ) Copyright (c) 1997-2018 The PHP Group Zend Engine v3.3.8, Copyright (c) 1998-2018 Zend Technologies with Zend OPcache v7.3.8-1+ubuntu18.04.1+deb.sury.org+1, Copyright (c) 1999-2018, by Zend Technologies

@erlangsec erlangsec reopened this Sep 21, 2019
zbateson added a commit that referenced this issue Sep 24, 2019
Rewrite regex to fix PHP7.3 bug in preg_split() #92
@zbateson
Copy link
Owner

Released in 1.1.6. Thanks for your help on this one!

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

No branches or pull requests

2 participants