multiple security vulnerabilities in taglib

[Lukáš Lalinský]

Thank you for the bug reports.

On Sun, Mar 4, 2012 at 4:41 AM, Zubin Mithra <zubin.mithra at gmail.com> wrote:
> - Sanity checks are not performed for fields read from a media file, which
> are used to allocate memory later on. Causes DoS due to application crash at
> the very least, exploitability is unconfirmed.
>
> An example :-
> apeitem.cpp
>   APE::Item::parse(const ByteVector &data)
>     d->key = String(data.mid(8), String::UTF8);

This doesn't necessarily cause problems, because the data array only
contains real data from the file. If the file is too large, I think
it's reasonable that it tries to allocate such large string.

On the other hand, maybe I overlooked something. If so, could you
please provide a file where this happens.

> - ogg/xiphcomment.cpp, Ogg::XiphComment::parse(const ByteVector &data)
>     Control over "vendorLength" and can cause a string allocation with that
> size. Control over "commentFields" which is the number of times,
> "commentLength" is read and a string of size "commandLength" is allocated.
> Causes DoS due to application crash at the very least, exploitability is
> unconfirmed.

The same here, it won't try to allocate a string that is larger than
the actual data length. So this happens only for files that are
actually that large, simply modifying the length field in the file
will not cause problems.

There was one related bug, if the file was too large, it would read
the vendorLength incorrectly. Fixed in
https://github.com/taglib/taglib/commit/ab8a0ee8937256311e649a88e8ddd7c7f870ad59

> - ape/apeproperties.cpp, APE::Properties::analyzeCurrent()
>     Specially crafted ape media files with sampleRate being "0" could lead
> to application crash, division by zero error.
>
>     d->sampleRate = header.mid(20, 4).toUInt(false);
>     d->length = totalBlocks / d->sampleRate;

Fixed in https://github.com/taglib/taglib/commit/77d61c6eca4d08b9b025738acf6b926cc750db23

> - crafted ogg file with a 1 bit change(0=>1) at the offset 0x0000007f leads
> to an infinite loop in the thread processing the tags.

I have trouble reproducing this. Could you send me the modified file
to lalinsky at gmail.com?

Lukas