[poppler] 13 commits - fofi/FoFiType1C.cc glib/poppler-action.cc glib/poppler-document.cc glib/poppler-page.cc makefile.vc poppler/ArthurOutputDev.cc poppler/CairoOutputDev.cc poppler/CharCodeToUnicode.cc poppler/Error.cc poppler/Gfx.cc poppler/G

[Kristian Høgsberg]

On 9/24/07, Krzysztof Kowalczyk <kjk at kemper.freedesktop.org> wrote:
5a1f670a4d16affeed86cdf643ab22f481caa3a5)
> Author: Krzysztof Kowalczyk <kkowalczyk at tlapx60ubu.(none)>
> Date:   Sun Sep 23 22:28:16 2007 -0700
>
>     replace extremely confusing 'a*(int)sizeof(foo)/sizeof(foo) != a' which, due to type promotions, if a is int, is equivalent to a < 0; fix problems revealed by the change

NO! Revert this commit right away.  Those checks are integer overflow
checks and prevent a class of security bugs.  The problem is that when
a pdf document specifies two (or more) numbers that poppler has to
multiply to determine how much memory to use, integer overflows can
happen.  For example, suppose the document specifies the width and
height of an image.  Both width and height can be positive, innocent
looking numbers, but when multiplied together, the results overflow
32bits.  The result may be a small positive integer, say 17 bytes,
which poppler then successfully allocates and then starts reading the
(huge) image data into that 17 byte allocation.  What you have is an
exploitable security bug, and if you check the security history of
xpdf and poppler, you'll see that we've fixed a great many of those
over time.

A nice patch would be to introduce a macro such as CHECK_OVERFLOW(a,b)
that implements the above check, which makes the code more readable
and isolates the implementation of this tricky check in place.  But
the checks have to stay.

cheers,
Kristian