Bug#342292: Fwd: Re: [vendor-sec] xpdf update - patch wrong?
* Frank Küster:
> The function that is called in *tetex-bin* is not gmallocn, but gmalloc
> - it's based on xpdf 3.00, not 3.01, and this is the very reason why I
> need to check for an overflow in nTiles * sizeof(JPXTile).
Sure, I wanted to explain why this is not sufficient. It should be
equivalent to the gmallocn check (once that is fixed, as discussed).
>> By the way, the gmallocn function suffers from undefined integer
>> overflow, too:
>>
>> void *gmallocn(int nObjs, int objSize) {
>> int n;
>>
>> n = nObjs * objSize;
>> if (objSize == 0 || n / objSize != nObjs) {
>> fprintf(stderr, "Bogus memory allocation size\n");
>> exit(1);
>> }
>> return gmalloc(n);
>> }
>
> What's the problem here? That the value in "n" is undefined, and
> therefore the comparison n / objSize != nObjs is undefined, too?
Exactly. You have a strange way of learning C, most programmers learn
that signed integer overflow is undefined only after they've written
tens of thousands of lines of code. 8-)
> This xpdf stuff seems to be a security nightmare by itself, even if not
> copied to everybodies orig.tar.gz.
The xpdf code which we are discussing is pretty much industry
standard, I fear.
>
>> The error handling is not suitable for library use, either. I don't
>> know if this is a problem.
>
> Only if the poppler people didn't notice...
According to their CVS repository, they didn't. 8-(
I'm going to notify them. I'm also going to report the undefined
behavior problem to the xpdf folks.
Reply to: