Bug 694880 - Seg faults found by fuzzing in opj_j2k_add_tlmarker (j2k.c:6147)
Summary: Seg faults found by fuzzing in opj_j2k_add_tlmarker (j2k.c:6147)
Status: RESOLVED FIXED
Alias: None
Product: MuPDF
Classification: Unclassified
Component: fuzzing (show other bugs)
Version: master
Hardware: PC Linux
: P4 minor
Assignee: Robin Watts
URL:
Keywords: bountiable
Depends on:
Blocks:
 
Reported: 2014-01-08 21:04 UTC by Marcos H. Woehrmann
Modified: 2014-02-25 16:15 UTC (History)
2 users (show)

See Also:
Customer:
Word Size: ---


Attachments
log.txt (570.07 KB, text/plain)
2014-01-08 21:04 UTC, Marcos H. Woehrmann
Details
Patch to prevent valgrind errors (1.55 KB, patch)
2014-01-24 11:51 UTC, Shailesh Mistry
Details | Diff
alternative patch (1.36 KB, patch)
2014-01-25 15:25 UTC, zeniko
Details | Diff
git format-patch (1.66 KB, patch)
2014-02-14 09:30 UTC, zeniko
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Marcos H. Woehrmann 2014-01-08 21:04:13 UTC
Created attachment 10520 [details]
log.txt

Seg faults in the 64 bit build of mupdf were found by fuzzing in opj_j2k_add_tlmarker (j2k.c:6147) while reading these files. See the attached log.txt for details.

00a030484bc3430dc0da95d792f74915_signal_sigsegv_86190e_7852_6278.pdf.pgmraw.72.0
082731e0bbb57637a4c4183923749bbe_asan_heap-oob_130deb6_3062_3057.pdf.ppmraw.200.0
a5d29876e5afcd091521886c5455b796_signal_sigsegv_7a3f29_4561_4064.pdf.ppmraw.200.0
Comment 2 Robin Watts 2014-01-20 06:28:20 UTC
Even with the suggested fix applied the following command gives Valgrind issues.

sudo valgrind build/release/mudraw -d -r72 -o /dev/null /home/marcos/cluster/tests_private/fuzzing/mupdf2/00a030484bc3430dc0da95d792f74915_signal_sigsegv_86190e_7852_6278.pdf

==29929== Invalid write of size 8
==29929==    at 0xAD4200: opj_j2k_add_tlmarker.isra.14 (in /home/robin/sauce/mupdf.git/build/release/mudraw)
==29929==    by 0xAD7F3C: opj_j2k_read_tile_header (in /home/robin/sauce/mupdf.git/build/release/mudraw)
==29929==    by 0xAD8A95: opj_j2k_decode_tiles (in /home/robin/sauce/mupdf.git/build/release/mudraw)
==29929==    by 0xAD3423: opj_j2k_exec (in /home/robin/sauce/mupdf.git/build/release/mudraw)
==29929==    by 0xAD9F67: opj_j2k_decode (in /home/robin/sauce/mupdf.git/build/release/mudraw)
==29929==    by 0xADD1FA: opj_jp2_decode (in /home/robin/sauce/mupdf.git/build/release/mudraw)
==29929==    by 0xA6D9C1: fz_load_jpx (in /home/robin/sauce/mupdf.git/build/release/mudraw)
==29929==    by 0xA52564: pdf_load_jpx (in /home/robin/sauce/mupdf.git/build/release/mudraw)
==29929==    by 0xA51F45: pdf_load_image_imp (in /home/robin/sauce/mupdf.git/build/release/mudraw)
==29929==    by 0xA52751: pdf_load_image (in /home/robin/sauce/mupdf.git/build/release/mudraw)
==29929==    by 0xA59EBB: pdf_run_Do (in /home/robin/sauce/mupdf.git/build/release/mudraw)
==29929==    by 0xA553DE: pdf_run_keyword (in /home/robin/sauce/mupdf.git/build/release/mudraw)
==29929==  Address 0x68cade0 is not stack'd, malloc'd or (recently) free'd
==29929==
==29929== Invalid write of size 8
==29929==    at 0xAD8061: opj_j2k_read_tile_header (in /home/robin/sauce/mupdf.git/build/release/mudraw)
==29929==    by 0xAD8A95: opj_j2k_decode_tiles (in /home/robin/sauce/mupdf.git/build/release/mudraw)
==29929==    by 0xAD3423: opj_j2k_exec (in /home/robin/sauce/mupdf.git/build/release/mudraw)
==29929==    by 0xAD9F67: opj_j2k_decode (in /home/robin/sauce/mupdf.git/build/release/mudraw)
==29929==    by 0xADD1FA: opj_jp2_decode (in /home/robin/sauce/mupdf.git/build/release/mudraw)
==29929==    by 0xA6D9C1: fz_load_jpx (in /home/robin/sauce/mupdf.git/build/release/mudraw)
==29929==    by 0xA52564: pdf_load_jpx (in /home/robin/sauce/mupdf.git/build/release/mudraw)
==29929==    by 0xA51F45: pdf_load_image_imp (in /home/robin/sauce/mupdf.git/build/release/mudraw)
==29929==    by 0xA52751: pdf_load_image (in /home/robin/sauce/mupdf.git/build/release/mudraw)
==29929==    by 0xA59EBB: pdf_run_Do (in /home/robin/sauce/mupdf.git/build/release/mudraw)
==29929==    by 0xA553DE: pdf_run_keyword (in /home/robin/sauce/mupdf.git/build/release/mudraw)
==29929==    by 0xA5619B: pdf_run_stream (in /home/robin/sauce/mupdf.git/build/release/mudraw)
==29929==  Address 0x68cade8 is not stack'd, malloc'd or (recently) free'd
==29929==
==29929== Invalid write of size 8
==29929==    at 0xAD806D: opj_j2k_read_tile_header (in /home/robin/sauce/mupdf.git/build/release/mudraw)
==29929==    by 0xAD8A95: opj_j2k_decode_tiles (in /home/robin/sauce/mupdf.git/build/release/mudraw)
==29929==    by 0xAD3423: opj_j2k_exec (in /home/robin/sauce/mupdf.git/build/release/mudraw)
==29929==    by 0xAD9F67: opj_j2k_decode (in /home/robin/sauce/mupdf.git/build/release/mudraw)
==29929==    by 0xADD1FA: opj_jp2_decode (in /home/robin/sauce/mupdf.git/build/release/mudraw)
==29929==    by 0xA6D9C1: fz_load_jpx (in /home/robin/sauce/mupdf.git/build/release/mudraw)
==29929==    by 0xA52564: pdf_load_jpx (in /home/robin/sauce/mupdf.git/build/release/mudraw)
==29929==    by 0xA51F45: pdf_load_image_imp (in /home/robin/sauce/mupdf.git/build/release/mudraw)
==29929==    by 0xA52751: pdf_load_image (in /home/robin/sauce/mupdf.git/build/release/mudraw)
==29929==    by 0xA59EBB: pdf_run_Do (in /home/robin/sauce/mupdf.git/build/release/mudraw)
==29929==    by 0xA553DE: pdf_run_keyword (in /home/robin/sauce/mupdf.git/build/release/mudraw)
==29929==    by 0xA5619B: pdf_run_stream (in /home/robin/sauce/mupdf.git/build/release/mudraw)
==29929==  Address 0x68cadf0 is not stack'd, malloc'd or (recently) free'd

Debug and Memento builds show errors too (the first is the same, the second two are different).
Comment 3 Henry Stiles 2014-01-22 09:01:24 UTC
Make bug bountiable with minor severity.  See http://ghostscript.com/Bug_bounty_program.html for our policy change on minor and trivial problems.
Comment 4 Shailesh Mistry 2014-01-24 11:51:27 UTC
Created attachment 10614 [details]
Patch to prevent valgrind errors

This patch caters for all the valgrind errors reported for 00a030484bc3430dc0da95d792f74915_signal_sigsegv_86190e_7852_6278.pdf
Comment 5 zeniko 2014-01-25 15:25:16 UTC
Created attachment 10617 [details]
alternative patch

The above patch fixes all locations where current_nb_tps >= current_nb_tps. Wouldn't a proper fix be to make sure that this is never the case in the first place, though?

The reason I haven't been able to reproduce this issue is that we've had one additional patch in our tree where I wasn't sure what is was needed for. Turns out that it fixes this issue.

Shelly: Does this patch also fix all Valgrind warnings in this bug and in bug 694904?
Comment 6 zeniko 2014-02-14 09:30:00 UTC
Created attachment 10697 [details]
git format-patch
Comment 7 Henry Stiles 2014-02-25 09:05:58 UTC
Assigning to Robin for mupdf integration, fixed with:

commit 863a1dfb328298a39871e49d218a16781fd42cba
Author: Simon Bünzli <zeniko@gmail.com>
Date:   Fri Feb 14 18:16:55 2014 +0100

    Bug 694880 and bug 694904: prevent heap overflow in opj_j2k_add_tlmarker

    Signed-off-by: Henry Stiles <henry.stiles@artifex.com>
Comment 8 Robin Watts 2014-02-25 16:15:37 UTC
The fix for this has now been pulled into the MuPDF repo.