Bug 697551

Summary:	mujstest: stack-based buffer overflow in man (jstest_main.c)
Product:	MuPDF	Reporter:	Agostino Sarubbo <ago>
Component:	apps	Assignee:	MuPDF bugs <mupdf-bugs>
Status:	RESOLVED FIXED
Severity:	normal	CC:	sebastian.rasmussen
Priority:	P4
Version:	1.10
Hardware:	PC
OS:	Linux
Customer:		Word Size:	---
Attachments:	stacktrace

Description Agostino Sarubbo 2017-02-05 12:53:17 UTC

Created attachment 13355 [details]
stacktrace

Tested on: 1.10a

To reproduce:
# mujstest $FILE

Reproducer:
https://github.com/asarubbo/poc/blob/master/00147-mupdf-mujstest-stackoverflow-main

Comment 1 Sebastian Rasmussen 2017-04-08 19:58:35 UTC

This was resolved in cfe8f35bca61056363368c343be36812abde0a06.

Comment 2 Agostino Sarubbo 2017-04-10 07:35:11 UTC

(In reply to Sebastian Rasmussen from comment #1)
> This was resolved in cfe8f35bca61056363368c343be36812abde0a06.

No, this patch fixed the strcpy-param-overlap issue, but introduced the stack buffer overflow issue.

Comment 3 Sebastian Rasmussen 2017-04-10 20:13:42 UTC

(In reply to Agostino Sarubbo from comment #2)
> (In reply to Sebastian Rasmussen from comment #1)
> > This was resolved in cfe8f35bca61056363368c343be36812abde0a06.
> 
> No, this patch fixed the strcpy-param-overlap issue, but introduced the
> stack buffer overflow issue.

That's not entirely true. The stack buffer overflow was being masked by the issue my patch resolved. I did run your file successfully without issues in both valgrind and gdb before setting this bug as resolved. Prompted by your reponse I figured out how to run asan using gcc and was finally able to see the problem you mentioned.

I have a tentative fix here (which may or may not go into 1.11):
http://git.ghostscript.com/?p=user/sebras/mupdf.git;a=commit;h=06a012a42c9884e3cd653e7826cff1ddec04eb6e

As well as a related cleanup here:
http://git.ghostscript.com/?p=user/sebras/mupdf.git;a=commit;h=e089b2e2c1d38c5696c7dfd741e21f8f3ef22b14

Comment 4 Agostino Sarubbo 2017-04-29 06:42:07 UTC

https://github.com/google/sanitizers/wiki/AddressSanitizerComparisonOfMemoryTools

Valgrind does not report the stack buffer overflow, this is the reason because you didn't see the bug when you made a patch for the strcpy-param-overlap issue.

Comment 5 Sebastian Rasmussen 2017-04-29 09:32:37 UTC

As I anticipated the fixes 06a012a42c9884e3cd653e7826cff1ddec04eb6e and e089b2e2c1d38c5696c7dfd741e21f8f3ef22b14 never made it into 1.11, but is on master now and will be in the next release.