Comment 32 for bug 26650

Message-ID: <email address hidden>
Date: Thu, 08 Dec 2005 23:19:53 +0100
From: Florian Weimer <email address hidden>
To: Frank =?iso-8859-1?Q?K=FCster?= <email address hidden>
Cc: <email address hidden>, Martin Pitt <email address hidden>, Martin Pitt <email address hidden>
Subject: Re: Bug#342292: Fwd: Re: [vendor-sec] xpdf update - patch wrong?

* Frank K=FCster:

> 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 =3D nObjs * objSize;
>> if (objSize =3D=3D 0 || n / objSize !=3D 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 !=3D 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.