Re: [libextractor] Another security bug in libextractor

[Christian Grothoff]

On Monday 15 May 2006 14:48, Luigi Auriemma wrote:
> Hey,
>
> I want to report also another heap overflow and a common bug.
>
> The heap overflow happens in qtextractor.c exactly in this line
> (STSD_ATOM handling):
>
>         memcpy(trak->stsd + 12, &trak_atom[i + 0x0C + hack_adjust],
>           BE_32(&trak_atom[i + 0x0C + hack_adjust]));
>
> I think that here we can see also a normal bug since the instruction
> copies not only the data but also the size of the data (IMHO the second
> argument should be &trak_atom[i + 16 + hack_adjust] like in the
> current Xine version) and naturally the heap overflow happens since
> trak->stsd has been previously allocated with current_atom_size but
> here we copy an amount of data specified in another "size" ever
> provided by the input file.
>
> I will write a complete proof-of-concept tomorrow but the alfa version
> I have written some minutes ago fully confirms the vulnerability.

No need.  I've started a complete rewrite of the qtextractor from scratch.  
The old xine code is just far, far too ugly and I've finally some half-decent 
documentation available.  The code is not done yet, but I think it'll be more 
capeable, easier to extend and maintain and much safer (no hacks!). 

> The other normal bug instead is in src/plugins/riffextractor.c:
>
>   if (c > 8) {
>     word = malloc(c+1-8);
>     memcpy(word,
>          &buffer[8],
>          c);
>     word[c-8] = '\0';
>
> If c is 10 we allocate 3 bytes for word but then we copy 10 bytes in it.

Well, and the malloced word is never used afterwards.  Fortunately, this 
entire block is in an #ifdef NEVER, so it is truly dead code so it does not 
really matter ;-). Fixed anyway.

Christian