BUG: Yara crash with one rule with one especific file

[GoogleCodeExporter]

Hi there! 
I left here the rule and the file that makes yara crash:

https://rapidshare.com/files/794370082/YARACRASH.zip

WARNING!!! The file contained in the ZIP can be malware, don't execute it.

The ZIP password is: yara

I tested it with Yara 1.6 152 (VS2008), yara 1.6 (mingw), and yara 1.5 under 
windows 7 x64, and crashed in all the cases. 

Greetings!

Original issue reported on code.google.com by golgotr...@gmail.com on 20 Jun 2012 at 6:42

[GoogleCodeExporter]

Just out of curiosity, I took a look at this. It seems that this is the problem:

    261                 while (mask[m] != MASK_OR && mask[m] != MASK_OR_END)
    262                 {
    263                     if ((buffer[tmp_b] & mask[m]) != pattern[p])
    264                     {
    265                         match = FALSE;
    266                     }
    267                     
    268                     if (match)
    269                     {
    270                         match_length++;
    271                     }
    272                 
    273                     tmp_b++;
    274                     m++;
    275                     p++;                    
    276                 }

There's no check that tmp_b is within the bounds of buffer[]. Most of the time, 
there's just more of the input file after this in memory, so it's not really a 
problem... until you reach the end of the file... AND... the file is exactly a 
multiple of your system's PAGE_SIZE in length, so that there's an unallocated 
page of memory immediately after the file (the buffer). The "YARACRASH" file is 
278528 bytes in length, which is 17*16384, so any memory allocations which are 
powers of two up to 2^14, are going to end exactly on the end of this file. 
There won't be a bunch of NULLs after it in memory. (Which, again, mostly won't 
cause a crash, it's just like the input file had a bunch of extra zeros at the 
end...)

Anyway, breaking the loop if (tmp_b < (size_t) buffer_size) seems to be the 
fix, and I tested this on just this one sample, and there's no more crash, but 
I don't know the full consequences of this change. I've uploaded a patch, 
apparently now called Issue 56 .

These are my notes:

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_INVALID_ADDRESS at address: 0x00000001000c4000
0x000000010000afc7 in hex_match (buffer=0x1000c3ffe "<m" <Address 0x1000c4000 
out of bounds>, buffer_size=2, pattern=0x1001002a0 "< iiI IfFrRaAmMeE", 
pattern_length=17, mask=0x1001002d0 "??????????????????????????????????????") 
at scan.c:263
263                         if ((buffer[tmp_b] & mask[m]) != pattern[p])
(gdb) print m
$9 = 3
(gdb) print p
$10 = 2
(gdb) print tmp_b
$11 = 2

(gdb) print buffer
$16 = (unsigned char *) 0x1000c3ffe "<m" <Address 0x1000c4000 out of bounds>
(gdb) x/16x 0x1000c3ff0
0x1000c3ff0:    0xfd    0x45    0x9f    0xea    0x24    0x11    0xa9    0x3d
0x1000c3ff8:    0x89    0xf8    0x18    0x0c    0x8c    0x19    0x3c    0x6d
(gdb) x/16x buffer
0x1000c3ffe:    0x3c    0x6d    Cannot access memory at address 0x1000c4000

hexdump -C YARACRASHER
00000000  4d 5a 90 00 03 00 00 00  04 00 00 00 ff ff 00 00  |MZ..............|
[...]
00043ff0  fd 45 9f ea 24 11 a9 3d  89 f8 18 0c 8c 19 3c 6d  |.E..$..=......<m|
00044000

Original comment by juliavi...@gmail.com on 26 Jun 2012 at 1:21

[GoogleCodeExporter]

Oh, and I was also going to say... If you're at all like me, and hate using 
rapidshare, this was the rule from "YARACRASH.ZIP":

rule yara_crash
{
    strings: 
            $a = {3C ( 20 69 |69 | 49 | 20 49) (66 | 46) (72 | 52) (61 | 41) (6D | 4D) (65 | 45)}  //"<iframe" nocase
    condition:
        $a 
}

And the EXE file... Really is malware:

https://www.virustotal.com/file/efdc6f5cb4f867e97f4b2fe437afef8a602455ea756ca67b
dea5a3fb9734dd44/analysis/

... But, as I said, I believe that the bug would be reproducible with any file 
that is exactly 278528 bytes long.

Original comment by juliavi...@gmail.com on 26 Jun 2012 at 1:25

[GoogleCodeExporter]

The bug was exactly what Julia described. Fixed in r153.

Original comment by plus...@gmail.com on 26 Jun 2012 at 1:25

[GoogleCodeExporter]

Original comment by plus...@gmail.com on 26 Jun 2012 at 1:26

• Changed state: Fixed

[GoogleCodeExporter]

So... I'm not familiar (yet) with the logic of this function, but should there 
also be a check for:
(p < (size_t) pattern_length)
around the same context as: 
p++;
or is mask[m] always guaranteed to be smaller than pattern[p] ?

Original comment by juliavi...@gmail.com on 27 Jun 2012 at 12:22

[GoogleCodeExporter]

pattern and mask are related, they are not necessarily the same length, but the 
function should not reach the end of mask without reaching the end of pattern 
at the same time.

Original comment by plus...@gmail.com on 27 Jun 2012 at 7:41

[GoogleCodeExporter]

Thank you very much for the bugfix! :DDDD

Original comment by golgotr...@gmail.com on 28 Jun 2012 at 5:55