[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

CVE-2018-6187 in mupdf



Hi,

I've had a look at CVE-2018-6187 in mupdf. After some investigations I
think that we can safely consider the Wheezy version as unaffected by
this issue.

* Issue summary:

AFAIK this issue is the remaining part of an older issue (partially) fixed
by 520cc26d18c9ee245b56e9e91f9d4fcae02be5f0. Its summary is very helpful
to understand the problem:

"When cleaning a pdf file, various lists (of pdf_xref_len length) are
defined early on. If a repair is triggered during the clean, this can
cause pdf_xref_len to increase causing an overrun if it isn't
reevaluated."

In fact, pdf_xref_len(ctx, doc) is not evaluated each time its value is
needed but rather locally stored as xref_len. If pdf_xref_len(ctx, doc)
changes while xref_len doesn't, this creates inconsistencies later leading
to overruns.

* Fix:

Update xref_len each time repair is triggered:

xref_len = pdf_xref_len(ctx, doc); /* May have changed due to repair */

Full upstream fixes:
http://git.ghostscript.com/?p=mupdf.git;h=3e30fbb7bf5efd88df431e366492356e7eb969ec
http://git.ghostscript.com/?p=mupdf.git;a=commit;h=520cc26d18c9ee245b56e9e91f9d4fcae02be5f0

* Wheezy situation:

The Wheezy code is very different from the one in more recent versions. In fact,
we also have a pdf_resize_xref function which updates the xref->len variable,
but IMO there's no potential issue here since xref->len never gets 'locally
copied', that is there is no local variable that should really be updated after
such a function call.

> $ ack --cc "xref->len"
> pdf/pdf_xref.c
> 176:    for (i = xref->len; i < newlen; i++)
> 184:    xref->len = newlen;
> 221:            if (ofs + len > xref->len)
> 272:    if (i0 < 0 || i0 + i1 > xref->len)
> 327:    if (size > xref->len)
> 332:    if (num < 0 || num >= xref->len)
> 335:            return fz_throw("object id (%d %d R) out of range (0..%d)", num, gen, xref->len - 1);
> 498:    for (i = 0; i < xref->len; i++)
> 504:                    if (xref->table[i].ofs <= 0 || xref->table[i].ofs >= xref->len || xref->table[xref->table[i].ofs].type != 'n')
> 542:                    xref->len = 0;
> 598:            for (i = 1; i < xref->len; i++)
> 649:            for (i = 0; i < xref->len; i++)
> 688:    printf("xref\n0 %d\n", xref->len);
> 689:    for (i = 0; i < xref->len; i++)
> 767:            if (numbuf[i] < 1 || numbuf[i] >= xref->len)
> 770:                    error = fz_throw("object id (%d 0 R) out of range (0..%d)", numbuf[i], xref->len - 1);
> 812:    if (num < 0 || num >= xref->len)
> 813:            return fz_throw("object out of range (%d %d R); xref size %d", num, gen, xref->len);
> 904:    if (num < 0 || num >= xref->len)
> 906:            fz_warn("object out of range (%d %d R); xref size %d", num, gen, xref->len);
>
> pdf/pdf_stream.c
> 12:     if (num < 0 || num >= xref->len)
> 238:    if (num < 0 || num >= xref->len)
> 268:    if (num < 0 || num >= xref->len)
>
> pdf/pdf_repair.c
> 160:            if (n >= xref->len)
> 377:    for (i = xref->len - 1; i >= 0; i--)
> 451:    for (i = 0; i < xref->len; i++)
>
> apps/pdfextract.c
> 205:            for (o = 0; o < xref->len; o++)
>
> apps/pdfshow.c
> 172:    for (i = 0; i < xref->len; i++)
>
> apps/pdfclean.c
> 77:     if (num < 0 || num >= xref->len)
> 107:    for (num = 1; num < xref->len; num++)
> 162:    for (num = 1; num < xref->len; num++)
> 226:    for (num = 0; num < xref->len; num++)
> 244:    xref->table = fz_calloc(xref->len, sizeof(pdf_xref_entry));
> 249:    for (num = 1; num < xref->len; num++)
> 267:    xref->len = newlen + 1;
> 270:    for (num = 1; num < xref->len; num++)
> 374:    for (num = 0; num < xref->len; num++)
> 600:    fprintf(out, "xref\n0 %d\n", xref->len);
> 601:    for (num = 0; num < xref->len; num++)
> 612:    obj = fz_new_int(xref->len);
> 642:    for (num = 0; num < xref->len; num++)
> 664:    for (num = 0; num < xref->len; num++)
> 724:    uselist = fz_calloc(xref->len + 1, sizeof(char));
> 725:    ofslist = fz_calloc(xref->len + 1, sizeof(int));
> 726:    genlist = fz_calloc(xref->len + 1, sizeof(int));
> 727:    renumbermap = fz_calloc(xref->len + 1, sizeof(int));
> 729:    for (num = 0; num < xref->len; num++)

(Also, needless to say, reproducer doesn't trigger anything in wheezy)

If nobody is against it I will mark this issue no-dsa with some comment
like 'minor issue, very unlikely to be affected'.

Regards,
 Hugo

-- 
             Hugo Lefeuvre (hle)    |    www.owl.eu.com
4096/ 9C4F C8BF A4B0 8FC5 48EB 56B8 1962 765B B9A8 BACA

Attachment: signature.asc
Description: PGP signature


Reply to: