Bug 697551 - mujstest: stack-based buffer overflow in man (jstest_main.c)
Summary: mujstest: stack-based buffer overflow in man (jstest_main.c)
Status: RESOLVED FIXED
Alias: None
Product: MuPDF
Classification: Unclassified
Component: apps (show other bugs)
Version: 1.10
Hardware: PC Linux
: P4 normal
Assignee: muPDF bugs
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-02-05 12:53 UTC by Agostino Sarubbo
Modified: 2017-04-29 09:32 UTC (History)
1 user (show)

See Also:
Customer:
Word Size: ---


Attachments
stacktrace (3.38 KB, text/plain)
2017-02-05 12:53 UTC, Agostino Sarubbo
Details

Note You need to log in before you can comment on or make changes to this bug.
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.