This appears to have been introduced in August/September. The affected versions are:

5.4.32+
5.5.16+
all versions of 5.6
7@20140901


Unaffected is:

5.4.31
5.5.15
and earlier.

Here's a much reduced test case:

<?php

$string = '';

// These two in any order
$string .= "\r\n";
$string .= "''''";

// Total string length > 8192
$string .= str_repeat(chr(rand(32, 127)), 8184);

// Ending in this string
$string .= "say";

$finfo = new finfo();
$type = $finfo->buffer($string);
var_dump($type);

?>

I've learnt a fair bit more about this.

The bug is triggered by this rule
(https://github.com/php/php-src/blob/master@%7B2015-02-06%7D/ext/fileinfo/tests/magic#L16237):

0	search/4096	"""
>&0	regex	.*"""$	Python script text executable
!:mime text/x-python

Specifically, while considering the continuation rule, during the
second call to file_softmagic() - during
file_ascmagic_with_encoding().

Say the buffer you're performing finfo on has a length of 9000 bytes.
At about line 247 in softmagic.c on current master, the OFFADD flag is
set for this rule, so ms->c.li[cont_level - 1].off is added to the
magic_set's offset, bringing its value from 0 to 9000. I think this is
the root cause, because the rule that is being continued should have
matched at a very early offset (a few bytes into the later test case's
string). So perhaps the c.li array is being filled out incorrectly by
the main rule, or incorrectly flushed?

So, ms->offset ends up being 9000 instead of 0.

Then, a little later, we end up calling mget() (softmagic.c, L1161)
with:
 - "offset" pulled from the magic_set, value 9000
 - "nbytes" is 9000 (as it has been the whole way through)
 - "o" is the original offset of 0

Then when we then call mcopy(), we do so with an nbytes and offset
*both* set to 9000. That seems to be where things finally unravel: at
about line 1082 of softmagic.c, we locate 'buf' 9000 bytes into the
haystack string, and last only 8192 bytes into it.

This seems to violate the comment at those lines:
 /* mget() guarantees buf <= last */

So, (ms->search.s_len = last - buf) overflows, and writes a huge
value.

The crash doesn't happen until magiccheck() tries to estrndup() this
size, a few lines later.

@dominic thanks for the further investigations. Your observations are correct. This misbehavior is introduced after this commit: http://git.php.net/?p=php-src.git;a=commit;h=eeaec70 

As a quick fix you can add these lines after the for loop

if (last < buf) {
    last = b;
}

However I don't believe that this piece of code is quite correct in the long term. In the mentioned patch I also don't see any invalid magic introduced. What I suspect is that the patch might be not complete as it's merged from an upper libmagic version. I'm also going to check what you've pointed out with the python part.

Thanks.

Dominic,

I think you've found it ... see here https://github.com/file/file/commit/eced9dbd4aa438de22ff453c723136beac41a558.patch ....

That should have been included into the original patch, but it's quite subtle to have been needed. With these lines commented out in the magic the test passes for me.

Thanks.

Anatol

@dominic,

please check one of the supplied patches. It's either 5.4/5 or 5.6/master.

Thanks.

I'd give feedback, but the patches are private for me ("You have no access to bug #68819").

(I'm using the password from the original bug submission to do edits like this.)

@dominic, please fetch them here

http://belski.net/uploads/my/phpz/bugz/bug68819_54.patch
http://belski.net/uploads/my/phpz/bugz/bug68819_56.patch

Thanks.

On master, I applied the 5.6/master patch, and confirmed the test cases now pass. However, I can still target other rules. Here's another one:

<?php

$string = 'try: except:';
$string .= str_repeat(' ', 8178);
$string .= 'say';

var_dump(strlen($string));

$finfo = new finfo();
$type = $finfo->buffer($string);
var_dump($type);

?>

I crafted this one to target the rule:

0	search/4096	try:
>&0	regex	\^\\s*except.*:	Python script text executable

(This from just grepping for search/4096 followed by a regex continuation).

Ok, i've two news ... :) For one - the same overflow behavior is to reproduce in file-5.22, that means - it's not a PHP issue, but the mainstream one. Decide which one is bad/good. 

This bug is present in at least the range of 5.19-5.22. To reproduce, use the data from your latest PHP snippet and set a breakpoint like this (in 5.22):

(gdb) b softmagic.c:1140 if buf > last

I've also checked the current libmagic master, and there the issue isn't present anymore. However the code base differs now far more (not even talking that it's not released yet), so i would not backport it. Instead, i'd probably a hotfix for our codebase. We might still try to backport something later after the current master (probably 5.23) is released, right now I'd rather ensure that the offset doesn't exceed bytecnt or vice versa. That is the key point. On 5.6, please try the following patch:

====== START =======
diff --git a/ext/fileinfo/libmagic/softmagic.c b/ext/fileinfo/libmagic/softmagic.c
index c8c72b2..0938ea6 100644
--- a/ext/fileinfo/libmagic/softmagic.c
+++ b/ext/fileinfo/libmagic/softmagic.c
@@ -1074,6 +1074,9 @@ mcopy(struct magic_set *ms, union VALUETYPE *p, int type, int indir,
                        if (bytecnt > nbytes) {
                                bytecnt = nbytes;
                        }
+                       if (offset > bytecnt) {
+                               offset = bytecnt;
+                       }
                        if (s == NULL) {
                                ms->search.s_len = 0;
                                ms->search.s = NULL;

====== END =======

With this, it should be working even without the data patch. While the data patch can be updated later.

Thanks.

@dominic, any update on this? Otherwise i'd rather go by commiting 3 lines from my last comment.

Thanks.

@ab I've just tested that three line patch, and it looks good. Works well for me.

I haven't had time to chase the issue upstream. I guess that would mean adding an issue at http://bugs.gw.com/? If they haven't done a release since fixing this, that suggests they might not be aware of it.

@dominic, thanks for checkin. I'm going to prepare a patch for this later this week.

Cheers.

If the fix is already published upstream, should we still keep this as private or can we merge it now?

This issue is still fixed in libmagic master only. To my observations on the vanilla "file" command, the issue is present between 5.19 and 5.22 at least. But in PHP it's a mix of 5.14 and 5.17 with some backported patches. Also, this is somehow mixed with another issue about the extensive backtracking in PHP/PCRE, whereby libmagic uses regex.

In short - I'd still prefer to keep this one private. ATM it still looks not like only an upstream issue, or at least PHP is much more sensitive than upstream in this case.

Thanks.

I am having this issue on PHP 5.4.16 (CentOS yum install)
is there a way for me to build a magic file to use?
thank you.

About CVE:

"Use CVE-2015-4604 for the violation of the "mget() guarantees buf <=
last" constraint suggested in the [2015-02-05 13:53 UTC] comment.

Use CVE-2015-4605 for the issue in which offset can exceed bytecnt,
suggested in the [2015-02-09 17:10 UTC] comment.

These might be conceptually overlapping discoveries, but we decided to
have the two CVE IDs."