Bug 135393

Created attachment 105059 [details]
Patch for CAN-2004-0888

Created attachment 105228 [details]
bad1.pdf demo exploit pdf file.

Created attachment 105229 [details]
bad2.pdf demo exploit pdf file

Created attachment 105230 [details]
bad3.pdf demo exploit pdf file

Created attachment 105231 [details]
bad4.pdf demo exploit pdf file

Created attachment 105232 [details]
bad5.pdf demo exploit pdf file

removing embargo

In a patch for this issue in xpdf-3.00-3.4 from fc2 there is a comment
in Catalog.cc which says:

  // The gcc doesnt optimize this away, so this check is ok,
  // even if it looks like a pagesSize != pagesSize check.
  if (pagesSize*sizeof(Page *)/sizeof(Page *) != pagesSize ||
      pagesSize*sizeof(Ref)/sizeof(Ref) != pagesSize) { ... }

I found that at least with some versions of gcc this is actually
not the case.  If a variable in question was just assigned to then
optimizer indeed leaves that alone.  Otherwise the whole test,
and a block guarded by it, may disappear.  If an auxilliary
variable is used, to which something like pagesSize*sizeof(Page *)
is assigned, then the test is not optimized away.  The same,
of course, applies to assorted tests in XRef.cc

In any case depending in security patches that some particular
version of an optimizer will not screw us up does not seem like
a good plan.

A version of gcc where I run into that was gcc-2.96-113.  At least
RHEL2.1 may be affected and who knows what else.

it's a know issue, i have added a workaround in xpdf for rhel2.1 and
rhel3 and it works fine.

An errata has been issued which should help the problem 
described in this bug report. This report is therefore being 
closed with a resolution of ERRATA. For more information
on the solution and/or where to find the updated files, 
please follow the link below. You may reopen this bug report 
if the solution does not work for you.

http://rhn.redhat.com/errata/RHSA-2004-592.html

The patch to fix CAN-2004-888 seems to be broken on all 64bit
platforms (tested only on ia64 though), because of the fact that the
overflow checking:

if (size*sizeof(XRefEntry)/sizeof(XRefEntry) !=  size) {
  ... error ...
}

won't succeed, because the left side of the condition is evaluated as
64bit integer by default. (all the arithmetic in the expression is
done in the widest integer type, which is 64bit, because of
sizeof(XRefEntry), even if "size" is 32bit so the explicit casting in
the numerator of the fraction has to be added).

In case of memory allocation:

entries = (XRefEntry *)gmalloc(size * sizeof(XRefEntry));

it's truncated to 32bits as prototype of gmalloc is:

void *gmalloc(int);

so the overflow check won't detect the overflow presented by bad5.pdf
and leads to segfault, because gmalloc succeeds with an allocation
request of 8 bytes:

Program received signal SIGSEGV, Segmentation fault.
XRef (this=0x6000000000279080, strA=0x6000000000278f20, ownerPassword=0x0,
userPassword=0x0) at XRef.cc:89
89            entries[i].used = gFalse;
Current language:  auto; currently c++
(gdb) p size
$1 = 357913942
(gdb) p sizeof(XRefEntry)
$2 = 12
(gdb) p size*sizeof(XRefEntry)
$3 = 4294967304
(gdb) p size*sizeof(XRefEntry)&((1<<32)-1)
$4 = 8

Created attachment 110599 [details]
The fixed patch with explicit casting (its teTeX variant).

Wait a minute -- this patch drops a check from the last hunk:

  pagesSize*sizeof(Ref)/sizeof(Ref) != pagesSize

Surely that's not intentional?

Isn't this check in the first hunk where Catalog.cc is patched?

Sorry, I meant the last hunk of the interdiff.

yes, it's missing in the obove patch. it should look:
@@ -186,6 +192,11 @@
       }
       if (start >= pagesSize) {
        pagesSize += 32;
+        if (pagesSize*(int)sizeof(Page *)/sizeof(Page *) != pagesSize ||
+            pagesSize*(int)sizeof(Ref)/sizeof(Ref) != pagesSize) {
+          error(-1, "Invalid 'pagesSize' parameter.");
+          goto err3;
+        }

i have added the fix in xpdf-2.02-9.6 and xpdf-0.92-14.

A new CVE id has been assigned to instances which have the CAN-2004-0888
incomplete fix.  The new CVE id is CAN-2005-0206.

An advisory has been issued which should help the problem
described in this bug report. This report is therefore being
closed with a resolution of ERRATA. For more information
on the solution and/or where to find the updated files,
please follow the link below. You may reopen this bug report
if the solution does not work for you.

http://rhn.redhat.com/errata/RHSA-2005-213.html