On 9/24/07, Krzysztof Kowalczyk <[EMAIL PROTECTED]> wrote: 5a1f670a4d16affeed86cdf643ab22f481caa3a5) > Author: Krzysztof Kowalczyk <[EMAIL PROTECTED](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 _______________________________________________ poppler mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/poppler
