Project: chromium Issues People Development process History Sign in
New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 1 user
Status: Fixed
Owner:
Last visit > 30 days ago
Closed: May 2015
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug-Security



Sign in to add a comment
Security: Heap Overflow Vulnerability in JBIG2 handling, used by PDF Reader
Reported by mla...@gmail.com, May 3 2015 Back to list
VULNERABILITY DETAILS

An Heap Overflow can be triggered in the PDF Reader process due to improper
validation in the CJBig2_Image::expand function (pdfium).

Attempting to expand an image to a size which will overflow a 32bit integer
(2^32) is not detected and can result to a CJBig2_Image with a small buffer
and an important height size. 64-bits platforms are also affected because
JBig2_Realloc used a DWORD (32bit) for the size to allocate.

Later JBIG2 image operation can then allow an attacker to alter memory after
the allocated buffer. Due to these JBIG2 image manipulation routines, it is
possible to:
- Modify memory at any given position and up to 4GB after the buffer
- Modify memory at several distinct positions
- Only modify part of a byte using available bitwise operators (OR, AND, XOR)

Exploitation of this vulnerability to execute code in the context of the PDF
reader process is highly probable.

The fix is to validate that stride*height does not overflow 2^31 as it is
already done in the CJBig2_Image constructor. A patch is included.


VERSION

Chrome Version: All Chrome versions with pdfium PDF reader included.
Operating System: Tested on Linux and Windows.
Both 32-bits and 64-bits platforms are affected.


REPRODUCTION CASE

The jbig2-exploit.py POC script can be used to generate PDF files which
replace 8 bytes of memory with 'A' (0x41) at various positions.

The jbig2-overflow-4.pdf has been generated by this script and modify the
heap 4 bytes after the allocated buffer (offset=6). On Linux, this alter
glibc malloc structures.

$ ./jbig2-exploit.py jbig2-overflow-4.pdf 6
Generating PDF jbig2-overflow-4.pdf...
* initial image: 32x4 (stride=4)
* GenericRegion copy at line 1073741828
* Image will be reallocated with 20 bytes but size will be 32x1073741829
* Clear 8 bytes at offset 24 using NUL image and OR operation
* Set 8 bytes to 0x41 (A) at offset 24 using AND operation
* JBIG2 page: 30 bytes
* JBIG2 symbols: 144 bytes
* Generated PDF: 1148 bytes

$ ./pdfium_test jbig2-overflow-4.pdf
Rendering PDF file jbig2-overflow-4.pdf.
Non-linearized path...
*** Error in `./pdfium_test': malloc(): memory corruption: 0x0000000003117210 ***
Aborted (core dumped)

$ gdb pdfium_test core
...
(gdb) bt
#0  0x00007fac6a204cc9 in __GI_raise (sig=sig@entry=6)
    at ../nptl/sysdeps/unix/sysv/linux/raise.c:56
#1  0x00007fac6a2080d8 in __GI_abort () at abort.c:89
#2  0x00007fac6a241394 in __libc_message (do_abort=1, 
    fmt=fmt@entry=0x7fac6a34fb28 "*** Error in `%s': %s: 0x%s ***\n")
    at ../sysdeps/posix/libc_fatal.c:175
#3  0x00007fac6a24ef36 in malloc_printerr (ptr=0x3117210, 
    str=0x7fac6a34bc9a "malloc(): memory corruption", action=<optimized out>)
    at malloc.c:4996
#4  _int_malloc (av=av@entry=0x7fac6a58c760 <main_arena>, bytes=bytes@entry=72)
    at malloc.c:3447
#5  0x00007fac6a2512cc in __libc_calloc (n=<optimized out>, 
    elem_size=<optimized out>) at malloc.c:3219
#6  0x00000000004653d5 in CPDF_Jbig2Interface::JBig2_Malloc (this=0x310e9c8, 
    dwSize=72) at core/src/fxcodec/codec/codec_int.h:213
#7  0x00000000004ac421 in CJBig2_Object::operator new (size=72, pModule=0x310e9c8)
    at core/src/fxcodec/jbig2/JBig2_Object.cpp:23
...

Others errors can be generated using different offsets:

*** Error in `./pdfium_test': corrupted double-linked list: 0x0000000002c18910 ***
Aborted (core dumped)

*** Error in `./pdfium_test': free(): corrupted unsorted chunks: 0x0000000002e98910 ***
Aborted (core dumped)

*** Error in `./pdfium_test': free(): invalid next size (fast): 0x0000000002a54200 ***
Aborted (core dumped)

*** Error in `./pdfium_test': double free or corruption (out): 0x0000000002707240 ***
Aborted (core dumped)

...


 
jbig2-overflow-4.pdf
1.1 KB Download
jbig2-exploit.py
5.0 KB View Download
JBig2_Image-expand-validate.patch
614 bytes Download
Cc: tsepez@chromium.org
Labels: Cr-Internals-Plugins-PDF
Owner: jun_f...@foxitsoftware.com
Status: Assigned
Jun, That other bug to handle overflow in allocations in expand() functions should take care of this, right ? Lets fix this in all places, it is not good to not check failed allocations.
in c#1, i was talking about https://code.google.com/p/chromium/issues/detail?id=465435
Labels: Security_Impact-Stable Security_Severity-Medium M-44
Thank you, mlafon! And thanks for the patch. Note that for integer overflow, we use third_party/base/numerics to check for integer overflow in a consistent, canonical way.

    CheckedNumeric<DWORD> foo == ...;
    foo *= ...;
    malloc(foo.ValueOrDie());

(Not that we should be calling malloc directly either, of course; just to illustrate.)
Comment 4 by mla...@gmail.com, May 5 2015
The patch was made consistent with the existing code in core/src/fxcodec/jbig2/ and use the same test than the CJBig2_Image constructor but this can surely be improved to use CheckedNumeric.

Also note that the test does not only check for a possible overflow but also limit the number of allocated bytes to 100MB although I don't know the reason of this limit.
Project Member Comment 5 by clusterf...@chromium.org, May 6 2015
Labels: Pri-1
Cc: thestig@chromium.org steven...@foxitsoftware.com kai_j...@foxitsoftware.com
Status: Started
It's pending in https://codereview.chromium.org/1131023008/
Comment 8 by mla...@gmail.com, May 19 2015
Warning: There is also a potential signedness issue in CJBig2_Image::expand which can
still be triggered on 64-bit platform with the commited fix. It was the initial bug I
have found when fuzzing the JBIG2 parser but I forgot to mention it because it is not
exploitable and I later focused on the exploitable version (and my patch was prevented
it to be triggered by being more strict).

The problem is in the JBIG2_memset call done after the buffer reallocation:

  JBIG2_memset(m_pData + m_nHeight * m_nStride, v ? 0xff : 0, (h - m_nHeight)*m_nStride);

As h, m_nHeight and m_nStride are all FX_INT32 values, ((h - m_nHeight) * m_nStride) can
easily be made negative by using a proper h value. As JBIG2_memset uses a size_t
argument for the size, the negative number will be converted to a very big positive number
on 64-bit platform which will immediately crash the program.

For example, using the attached PDF (jbig2-memset-negative.pdf):

* h=1073741819, m_nStride=4, m_nHeight=4
* buffer will be reallocated to 4294967276 (= 4 * 1073741819, = 2^32 - 20)
* (h - m_nHeight) * m_nStride = 4294967260 (= 2^32 - 36)
* But as the operation is done using FX_INT32, (h - m_nHeight) * m_nStride = -36
* JBIG2_memset will so be called with sz=18446744073709551580 (2^64 - 36)

$ ./pdfium_test jbig2-memset-negative.pdf
Rendering PDF file jbig2-memset-negative.pdf.
Non-linearized path...
Segmentation fault (core dumped)

(gdb) bt 3
#0  memset () at ../sysdeps/x86_64/memset.S:80
#1  0x00000000004a99ed in CJBig2_Image::expand (this=0x32837a0, h=1073741819, v=0)
    at core/src/fxcodec/jbig2/JBig2_Image.cpp:780
#2  0x0000000000490966 in CJBig2_Context::parseGenericRegion (this=0x32834b0,
    pSegment=0x32837d0, pPause=0x0) at core/src/fxcodec/jbig2/JBig2_Context.cpp:1574
(More stack frames follow...)

(gdb) disas
...
   0x00007f7604afc614 <+84>:    movdqu %xmm8,(%rdi)
   0x00007f7604afc619 <+89>:    and    $0xffffffffffffffc0,%rcx
=> 0x00007f7604afc61d <+93>:    movdqu %xmm8,-0x10(%rdi,%rdx,1)
   0x00007f7604afc624 <+100>:   movdqu %xmm8,0x10(%rdi)
...

(gdb) print $rdx
$1 = -36

jbig2-memset-negative.pdf
1.1 KB Download
Status: started
Thanks for your reports. We will provide a new patch to cover the scenario in #8. 
Project Member Comment 11 by clusterf...@chromium.org, May 20 2015
Labels: -Restrict-View-SecurityTeam M-43 Restrict-View-SecurityNotify Merge-Triage
Adding Merge-Triage label for tracking purposes.

Once your fix had sufficient bake time (on canary, dev as appropriate), please nominate your fix for merge by adding the Merge-Requested label.

When your merge is approved by the release manager, please start merging with higher milestone label first. Make sure to re-request merge for every milestone in the label list. You can get branch information on omahaproxy.appspot.com.

- Your friendly ClusterFuzz
Project Member Comment 12 by bugdroid1@chromium.org, Jun 3 2015
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6e5d15268c5d75ba15189ce0a6050845068eb06b

commit 6e5d15268c5d75ba15189ce0a6050845068eb06b
Author: tsepez <tsepez@chromium.org>
Date: Wed Jun 03 00:50:49 2015

Roll PDFium to b29338d

This brings in:
b29338d Fix windows compile: fix size_t vs. int mismatch
e06b686 kill IPDF_DocParser().
4ff7a42 Fix heap use after free in Document::DoFieldDelay and Document::delay
8e1b608 Add missing comma to third_party.gyp
cafa3fd Run V8 in predictable mode for pdfium_test
8ba4a3c Fix suppressions for 2015-05-28 drop
878b819 Roll DEPS to pick up 2015-05-28 corpus drop.
6b776fe Fix ALL the include guards.
14f57a1 Remove rendundant ../include from paths of files in include/ directory
cddfde0 Upgrade openjpeg to r3002
5f566b3 Update copy of safe_math_impl.h to take a fix from upstream:
e6406b3 Fix four annoying warnings: Two "set but unused".
bc4b82e Fix an endless loop in CJBig2_HuffmanTable::parseFromCodedBuffer
79569e7 Get test running scripts to detect and report common error.
e9ccc9b Integer overflow in CJBig2_Image::expand
3a25130 Tidy public fpdfview.h and fpdf_flatten.h.
b190fc2 Turn on warnings for usage of disabled V8 APIs
981a346 Re-land: Remove FX_Alloc() null checks now that it can't return NULL.
bf4aa2c Revert "Remove FX_Alloc() null checks now that it can't return NULL."
eb65277 Remove FX_Alloc() null checks now that it can't return NULL.
59f4b44 Fix Heap Overflow in CJBig2_Image::expand
3b60890 Cleanup if early return from opj_j2k_copy_default_tcp_and_create_tcd().
3fea540 Replace v8::Handle with v8::Local and v8::Persistent with v8::Global
0c94bc4 Change FX_Alloc to FX_Try_Alloc in _JpegEncode
31b3a2b Add safe FX_Alloc2D() macro
a88e3a1 Add myself to OWNERS file
d94df88 Replace deprecated with non-deprecated V8 APIs
1962d61 Fix leaks in embedder test's FlateEncode() usage and in FlateEncode().
69b4bc7 Disable allocation tests that hose the bot.
acae925 Initialize members of CPDF_TextPageFind class.
61ffad8 Fix leaks in the embedder tests themselves.
9f6f348 Abort on OOM by default in FX_Alloc().
dc0bd92 Remove FX_NEW_VECTOR() macros.
7f3b99a Fix potential UAF in ConcatInPlace.
b60617f Fix another batch of compiler warnings.

BUG= 459215 , 482639 , 483981 , 486538 , 487928 , 488302 
R=thestig@chromium.org

Review URL: https://codereview.chromium.org/1159433007

Cr-Commit-Position: refs/heads/master@{#332514}

[modify] http://crrev.com/6e5d15268c5d75ba15189ce0a6050845068eb06b/DEPS

Project Member Comment 13 by bugdroid1@chromium.org, Jun 3 2015
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/af1125ea286450ceecc23a37c6710bcf0b2d1ce6

commit af1125ea286450ceecc23a37c6710bcf0b2d1ce6
Author: engedy <engedy@chromium.org>
Date: Wed Jun 03 09:15:29 2015

Revert of Roll PDFium to b29338d (patchset #2 id:20001 of https://codereview.chromium.org/1159433007/)

Reason for revert:
Causes compile errors on "Linux GN Clobber" bot:

../../third_party/pdfium/core/src/fxcrt/fx_basic_memmgr_unittest.cpp:23:31:error: expression result unused [-Werror,-Wunused-value]
    EXPECT_DEATH_IF_SUPPORTED(FX_Alloc(int, kMaxIntAlloc), "");

Archived full build log: https://goo.gl/4OImxB.

Original issue's description:
> Roll PDFium to b29338d
>
> This brings in:
> b29338d Fix windows compile: fix size_t vs. int mismatch
> e06b686 kill IPDF_DocParser().
> 4ff7a42 Fix heap use after free in Document::DoFieldDelay and Document::delay
> 8e1b608 Add missing comma to third_party.gyp
> cafa3fd Run V8 in predictable mode for pdfium_test
> 8ba4a3c Fix suppressions for 2015-05-28 drop
> 878b819 Roll DEPS to pick up 2015-05-28 corpus drop.
> 6b776fe Fix ALL the include guards.
> 14f57a1 Remove rendundant ../include from paths of files in include/ directory
> cddfde0 Upgrade openjpeg to r3002
> 5f566b3 Update copy of safe_math_impl.h to take a fix from upstream:
> e6406b3 Fix four annoying warnings: Two "set but unused".
> bc4b82e Fix an endless loop in CJBig2_HuffmanTable::parseFromCodedBuffer
> 79569e7 Get test running scripts to detect and report common error.
> e9ccc9b Integer overflow in CJBig2_Image::expand
> 3a25130 Tidy public fpdfview.h and fpdf_flatten.h.
> b190fc2 Turn on warnings for usage of disabled V8 APIs
> 981a346 Re-land: Remove FX_Alloc() null checks now that it can't return NULL.
> bf4aa2c Revert "Remove FX_Alloc() null checks now that it can't return NULL."
> eb65277 Remove FX_Alloc() null checks now that it can't return NULL.
> 59f4b44 Fix Heap Overflow in CJBig2_Image::expand
> 3b60890 Cleanup if early return from opj_j2k_copy_default_tcp_and_create_tcd().
> 3fea540 Replace v8::Handle with v8::Local and v8::Persistent with v8::Global
> 0c94bc4 Change FX_Alloc to FX_Try_Alloc in _JpegEncode
> 31b3a2b Add safe FX_Alloc2D() macro
> a88e3a1 Add myself to OWNERS file
> d94df88 Replace deprecated with non-deprecated V8 APIs
> 1962d61 Fix leaks in embedder test's FlateEncode() usage and in FlateEncode().
> 69b4bc7 Disable allocation tests that hose the bot.
> acae925 Initialize members of CPDF_TextPageFind class.
> 61ffad8 Fix leaks in the embedder tests themselves.
> 9f6f348 Abort on OOM by default in FX_Alloc().
> dc0bd92 Remove FX_NEW_VECTOR() macros.
> 7f3b99a Fix potential UAF in ConcatInPlace.
> b60617f Fix another batch of compiler warnings.
>
> BUG= 459215 , 482639 , 483981 , 486538 , 487928 , 488302 
> R=thestig@chromium.org
>
> Committed: https://crrev.com/6e5d15268c5d75ba15189ce0a6050845068eb06b
> Cr-Commit-Position: refs/heads/master@{#332514}

TBR=thestig@chromium.org,tsepez@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 459215 , 482639 , 483981 , 486538 , 487928 , 488302 

Review URL: https://codereview.chromium.org/1162103004

Cr-Commit-Position: refs/heads/master@{#332579}

[modify] http://crrev.com/af1125ea286450ceecc23a37c6710bcf0b2d1ce6/DEPS

Project Member Comment 14 by bugdroid1@chromium.org, Jun 3 2015
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1203cc8c7e82ab31d99190ccd595e813ac7ab9f9

commit 1203cc8c7e82ab31d99190ccd595e813ac7ab9f9
Author: tsepez <tsepez@chromium.org>
Date: Wed Jun 03 21:26:06 2015

Roll PDFium to 7bb4d8d

This brings in:
7bb4d8d Fix fx_basic_memmgr_unittest.cpp under stricter GN rules
a76f557 Automated test case for 487928.
b29338d Fix windows compile: fix size_t vs. int mismatch
e06b686 kill IPDF_DocParser().
4ff7a42 Fix heap use after free in Document::DoFieldDelay and Document::delay
8e1b608 Add missing comma to third_party.gyp
cafa3fd Run V8 in predictable mode for pdfium_test
8ba4a3c Fix suppressions for 2015-05-28 drop
878b819 Roll DEPS to pick up 2015-05-28 corpus drop.
6b776fe Fix ALL the include guards.
14f57a1 Remove rendundant ../include from paths of files in include/ directory
cddfde0 Upgrade openjpeg to r3002
5f566b3 Update copy of safe_math_impl.h to take a fix from upstream:
e6406b3 Fix four annoying warnings: Two "set but unused".
bc4b82e Fix an endless loop in CJBig2_HuffmanTable::parseFromCodedBuffer
79569e7 Get test running scripts to detect and report common error.
e9ccc9b Integer overflow in CJBig2_Image::expand
3a25130 Tidy public fpdfview.h and fpdf_flatten.h.
b190fc2 Turn on warnings for usage of disabled V8 APIs
981a346 Re-land: Remove FX_Alloc() null checks now that it can't return NULL.
bf4aa2c Revert "Remove FX_Alloc() null checks now that it can't return NULL."
eb65277 Remove FX_Alloc() null checks now that it can't return NULL.
59f4b44 Fix Heap Overflow in CJBig2_Image::expand
3b60890 Cleanup if early return from opj_j2k_copy_default_tcp_and_create_tcd().
3fea540 Replace v8::Handle with v8::Local and v8::Persistent with v8::Global
0c94bc4 Change FX_Alloc to FX_Try_Alloc in _JpegEncode
31b3a2b Add safe FX_Alloc2D() macro
a88e3a1 Add myself to OWNERS file
d94df88 Replace deprecated with non-deprecated V8 APIs
1962d61 Fix leaks in embedder test's FlateEncode() usage and in FlateEncode().
69b4bc7 Disable allocation tests that hose the bot.
acae925 Initialize members of CPDF_TextPageFind class.
61ffad8 Fix leaks in the embedder tests themselves.
9f6f348 Abort on OOM by default in FX_Alloc().
dc0bd92 Remove FX_NEW_VECTOR() macros.
7f3b99a Fix potential UAF in ConcatInPlace.
b60617f Fix another batch of compiler warnings.

BUG= 459215 , 482639 , 483981 , 486538 , 487928 , 488302 
R=thestig@chromium.org

Committed: https://crrev.com/6e5d15268c5d75ba15189ce0a6050845068eb06b
Cr-Commit-Position: refs/heads/master@{#332514}

Review URL: https://codereview.chromium.org/1159433007

Cr-Commit-Position: refs/heads/master@{#332687}

[modify] http://crrev.com/1203cc8c7e82ab31d99190ccd595e813ac7ab9f9/DEPS

Cc: timwillis@chromium.org
Labels: -Merge-Triage Merge-Request-44
Merge requested to M44 (branch 2403)
Labels: -Merge-Request-44 Merge-Review-44 Hotlist-Merge-Review
[Automated comment] Reverts referenced in bugdroid comments, needs manual review.
Right, so as discussed with Tim yesterday, I need clarification on which CLs (in pdfium) are needed for Beta (M44).  We're not going to just roll everything listed above.

You're in luck, there is already a nice M44 pdfium branch set up (https://pdfium.googlesource.com/pdfium/+log/refs/heads/chromium/2403), which will be used for the merge.

So please let me know which CLs you're requesting to merge first.  Thanks!
Labels: -Merge-Review-44 -Hotlist-Merge-Review Merge-Approved-44
Excellent, thank you.

Merge approved for M44 (chromium/2403 pdfium branch).

Once those two CLs are in the branch, please post the last hash here, so I can change the M44 DEPs file.
Comment 20 by mla...@gmail.com, Jun 29 2015
It seems that the handling of this issue is hanged for no obvious reason as the merge of the two related commits has been approved on chromium/2403 but nothing is pushed on that branch.

Is there anything that is blocking the merge?

Thanks.
Labels: -M-43 reward-topanel
Tom / Jun - can you please land the CLs at #18 into the chromium/2403 PDFium branch listed in #17?

@mlafon - once that's done, then pennymac@ can update the M44 DEPs file and we should be done here. Also adding reward-topanel to take this report through the chrome reward program.
I'll merge this *now*
Both commits merged to M44. Just waiting for a DEPS roll now.
Labels: -Merge-Approved-44 merge-merged-2403
Thanks Lei!
Labels: Release-0-M44
This is a great report IMHO :)
Labels: CVE-2015-1279
Comment 28 by mla...@gmail.com, Aug 15 2015
It's been nearly a month since 44 was released and this bug was mentioned as $5500 worth in the release notes but this issue was not updated with the reward information.

How can I claim the reward, do I need to wait until I am contacted ?

Thanks.
Labels: -reward-topanel reward-5500 reward-unpaid
I'll make sure that someone in our finance department reaches out to you this week. If you don't hear from someone, please contact me directly at timwillis@ so that I can chase it down for you.

Thanks again for the great report - hope to see more from you!
Project Member Comment 30 by clusterf...@chromium.org, Aug 26 2015
Labels: -Restrict-View-SecurityNotify
Bulk update: removing view restriction from closed bugs.
Labels: -reward-unpaid reward-inprocess
Payment in process: Waiting on supplier details.
Labels: -reward-inprocess
Processing via our e-payment system takes ~7 days, but the reward should be on its way to you. Thanks again for your help!
Project Member Comment 33 by sheriffbot@chromium.org, Oct 1 2016
This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member Comment 34 by sheriffbot@chromium.org, Oct 2 2016
This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: allpublic
Sign in to add a comment