Bug#342292: Fwd: Re: [vendor-sec] xpdf update - patch wrong?

[Frank Küster <frank@kuesterei.ch>]

Florian Weimer <fw@deneb.enyo.de> wrote:

> * Frank Küster:
>
>> Would 
>>
>>       if (nTiles >= INT_MAX / sizeof(JPXTile) {
>> 	error(getPos(), "Bad tile count in JPX SIZ marker segment");
>> 	return gFalse;
>>
>> be okay?
>
> It might still be a DoS issue, I think.  Allocating arbitrary amounts
> of memory upon user request is usually a bad idea.  But gmallocn does
> not touch the memory it allocates, so even very large allocations are
> very cheap initially.

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).

> As long as you initialize the allocated data
> structure as you read more input, it should be a minor issue (because
> you need an enormous file size to cause problems on even slightly
> dated machines).

I have no idea what the code does, and I'm only starting to learn C and
know next to nothing about C++.  Somebody else should check.

> 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?

This xpdf stuff seems to be a security nightmare by itself, even if not
copied to everybodies orig.tar.gz.

> 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...

Regards, Frank
-- 
Frank Küster
Inst. f. Biochemie der Univ. Zürich
Debian Developer