Bug 699271 - Infinite Loop in fz_skip_space (source/pdf/pdf-xref.c)
Summary: Infinite Loop in fz_skip_space (source/pdf/pdf-xref.c)
Status: RESOLVED FIXED
Alias: None
Product: MuPDF
Classification: Unclassified
Component: mupdf (show other bugs)
Version: master
Hardware: PC All
: P4 normal
Assignee: MuPDF bugs
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-04-21 12:07 UTC by traceprobe
Modified: 2018-08-22 11:42 UTC (History)
3 users (show)

See Also:
Customer:
Word Size: ---


Attachments
poc for infinite loop (571 bytes, application/zip)
2018-04-21 12:07 UTC, traceprobe
Details
Patch (845 bytes, patch)
2018-06-10 12:34 UTC, M.J.G.
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description traceprobe 2018-04-21 12:07:44 UTC
Created attachment 15052 [details]
poc for infinite loop

In latest version (1.13.0) of MuPDF, there is an infinite loop in fz_skip_space function of (source/pdf/pdf-xref.c) file, which could be triggered by the POC below.

The issue happens in following code: in the do{...}while(1); loop, fz_peek_byte function (line 649) returns EOF and fz_read_byte fails to move "fz_stream" to next byte. People said that "EOF" equals -1, and in that case, "if (c > 32 && c != EOF)" is equivalent to "if (c > 32)". Do you mean "if (c > 32 || c != EOF)" here?


    // source/pdf/pdf-xref.c
    644 static void
    645 fz_skip_space(fz_context *ctx, fz_stream *stm)
    646 {
    647         do
    648         {
    649                 int c = fz_peek_byte(ctx, stm);
    650                 if (c > 32 && c != EOF)
    651                         return;
    652                 (void)fz_read_byte(ctx, stm);
    653         }
    654         while (1);
    655 }

The stack trace is as follows:
#0  0x00000000009be1bd in fz_peek_byte (ctx=ctx@entry=0x60e000000040, stm=stm@entry=0x608000000120) at include/mupdf/fitz/stream.h:400
#1  0x00000000009c60f6 in fz_skip_space (stm=0x608000000120, ctx=0x60e000000040) at source/pdf/pdf-xref.c:649
#2  pdf_xref_size_from_old_trailer (ctx=ctx@entry=0x60e000000040, doc=doc@entry=0x631000014800, buf=buf@entry=0x631000014980) at source/pdf/pdf-xref.c:694
#3  0x00000000009c98a9 in pdf_read_old_xref (buf=0x631000014980, doc=0x631000014800, ctx=0x60e000000040) at source/pdf/pdf-xref.c:836
#4  pdf_read_xref (ctx=ctx@entry=0x60e000000040, doc=doc@entry=0x631000014800, ofs=ofs@entry=565, buf=buf@entry=0x631000014980) at source/pdf/pdf-xref.c:1069
#5  0x00000000009cae58 in read_xref_section (ctx=ctx@entry=0x60e000000040, doc=doc@entry=0x631000014800, ofs=ofs@entry=565, buf=buf@entry=0x631000014980)
    at source/pdf/pdf-xref.c:1089
#6  0x00000000009cc1e1 in pdf_read_xref_sections (ctx=ctx@entry=0x60e000000040, doc=doc@entry=0x631000014800, ofs=<optimized out>, buf=buf@entry=0x631000014980, 
    read_previous=read_previous@entry=1) at source/pdf/pdf-xref.c:1157
#7  0x00000000009d004e in pdf_load_xref (ctx=ctx@entry=0x60e000000040, doc=doc@entry=0x631000014800, buf=buf@entry=0x631000014980) at source/pdf/pdf-xref.c:1213
#8  0x00000000009ec4a0 in pdf_init_document (ctx=ctx@entry=0x60e000000040, doc=<optimized out>) at source/pdf/pdf-xref.c:1351
#9  0x00000000009eec01 in pdf_open_document (ctx=<optimized out>, 
    filename=filename@entry=0x7fffffffdea5 "/u///fuzz/ncmupdf/output/poc/id:000006,src:000920,op:replay,rep:4,tm:1524303630,repid:65552.pdf")
    at source/pdf/pdf-xref.c:2287
#10 0x00000000004a23e3 in pdfshow_main (argc=2, argv=0x7fffffffdb40) at source/tools/pdfshow.c:281
#11 0x000000000040cf8e in main (argc=<optimized out>, argv=<optimized out>) at source/tools/mutool.c:127
Comment 1 traceprobe 2018-04-23 10:25:13 UTC
Steps to reproduce:
1) download mupdf-1.13.0-source.tar.gz
2) decompress and compile
3) download attached POC
4) execute: ./mutool show $POC
Comment 2 M.J.G. 2018-06-10 11:24:02 UTC
"if (c > 32 || c == EOF)" would make sense: return when there is a non-space as the next char or nothing left to read. Then all spaces (in fact: everything <=32) has been skipped.

Otherwise, ending a line with spaces gives an infinite loop.
Comment 3 M.J.G. 2018-06-10 12:34:38 UTC
Created attachment 15230 [details]
Patch
Comment 4 Carlo B 2018-08-16 07:03:03 UTC
(In reply to M.J.G. from comment #3)
> Created attachment 15230 [details]
> Patch

Hello M.J.G., I was wondering if this bug issue still not fix? Do we have additional reference to help us fix this bug? Thank you.

Castro B
CEO - Owner
https://alternatives.co/
Comment 5 M.J.G. 2018-08-17 12:05:30 UTC
(In reply to Carlo B from comment #4)
> (In reply to M.J.G. from comment #3)
> > Created attachment 15230 [details]
> > Patch
> 
> Hello M.J.G., I was wondering if this bug issue still not fix? Do we have
> additional reference to help us fix this bug? Thank you.
> 
> Castro B
> CEO - Owner
> https://alternatives.co/

The problem is still there in the current source code (without my patch).

I'm the maintainer of the Fedora package for mupdf, I've got no say over here. There is a proof of concept by the original poster and a patch by myself which fixes the infinite loop. Nothing more I can do.
Comment 6 Sebastian Rasmussen 2018-08-22 11:42:26 UTC
Fixed in

commit 2e43685dc8a8a886fc9df9b3663cf199404f7637
Author: Sebastian Rasmussen <sebras@gmail.com>
Date:   Tue Aug 21 19:07:57 2018 +0800

    Bug 699271: Fix eternal loop when skipping space before EOF.
    
    Thanks to Michael J Gruber for providing this oneliner.