New issue
Advanced search Search tips

Issue 842503 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 25
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 2
Type: Bug-Security

Blocking:
issue 62400



Sign in to add a comment

Security: Uninitialized Memory Read in CXFA_LayoutPageMgr::GetAvailHeight

Reported by stackexp...@gmail.com, May 13 2018

Issue description

VULNERABILITY DETAILS
An out-of-bounds-read (uninitialized-memory-read) issue was found in PDFium. Following log was produced by AddressSanitizer.

=================================================================
==9584==ERROR: AddressSanitizer: access-violation on unknown address 0xbebebec6 (pc 0x037419e3 bp 0x002deb18 sp 0x002dea60 T0)
    #0 0x37419e2 in CXFA_LayoutPageMgr::GetAvailHeight C:\pdfium\xfa\fxfa\parser\cxfa_layoutpagemgr.cpp:489
    #1 0x3714d67 in CXFA_LayoutProcessor::DoLayout C:\pdfium\xfa\fxfa\parser\cxfa_layoutprocessor.cpp:73
    #2 0x36577ba in CXFA_FFDocView::DoLayout C:\pdfium\xfa\fxfa\cxfa_ffdocview.cpp:94
    #3 0x362e0bf in CPDFXFA_Context::LoadXFADoc C:\pdfium\fpdfsdk\fpdfxfa\cpdfxfa_context.cpp:118
    #4 0x2c14ffa in FPDF_LoadXFA C:\pdfium\fpdfsdk\fpdf_view.cpp:265
    #5 0x1054269 in main C:\pdfium\samples\pdfium_test.cc:911
    #6 0x3c5b90a in __scrt_common_main_seh f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl:283
    #7 0x7677343c in BaseThreadInitThunk+0x11 (C:\Windows\syswow64\kernel32.dll+0x7dd7343c)
    #8 0x77569831 in RtlInitializeExceptionChain+0x62 (C:\Windows\SysWOW64\ntdll.dll+0x7dea9831)
    #9 0x77569804 in RtlInitializeExceptionChain+0x35 (C:\Windows\SysWOW64\ntdll.dll+0x7dea9804)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: access-violation C:\pdfium\xfa\fxfa\parser\cxfa_layoutpagemgr.cpp:489 in CXFA_LayoutPageMgr::GetAvailHeight
==9584==ABORTING


If we compile a normal version of pdfium (with is_asan=false and is_debug=false), and enable pageheap option for pdfium_test.exe, we'll get the following information. Here the value of eax was 0xc0c0c0c0 (read from a 12-bytes heap) which indicates that the value was not initialized.

=================================================================
(2c60.499c): Access violation - code c0000005 (!!! second chance !!!)
eax=c0c0c0c0 ebx=0aed0f90 ecx=0af76fc0 edx=0031f394 esi=0af76fc0 edi=0031f3bc
eip=01b19473 esp=0031f378 ebp=0031f3a0 iopl=0         nv up ei pl nz na po nc
cs=0023  ss=002b  ds=002b  es=002b  fs=0053  gs=002b             efl=00010202
pdfium_test!CXFA_LayoutPageMgr::GetAvailHeight+0x1d:
01b19473 8b4008          mov  eax,dword ptr [eax+8] ds:002b:c0c0c0c8=????????

0:000> r eax
eax=c0c0c0c0

0:000> ub eip
pdfium_test!CXFA_LayoutPageMgr::GetAvailHeight+0x5 
    [C:\pdfium\xfa\fxfa\parser\cxfa_layoutpagemgr.cpp @ 487]:
01b1945b 83ec20          sub     esp,20h
01b1945e a1b09bf801      mov     eax,dword ptr [pdfium_test!__security_cookie (01f89bb0)]
01b19463 89ce            mov     esi,ecx
01b19465 0f57c0          xorps   xmm0,xmm0
01b19468 31e8            xor     eax,ebp
01b1946a 8945f4          mov     dword ptr [ebp-0Ch],eax
01b1946d 8b4118          mov     eax,dword ptr [ecx+18h]
01b19470 8b4008          mov     eax,dword ptr [eax+8]

0:000> dd ecx+18 L1
0af76fd8  0af78ff0

0:000> dd 0af78ff0
0af78ff0  0af78ff0 0af78ff0 c0c0c0c0 d0d0d0d0
0af79000  ???????? ???????? ???????? ????????

0:000> !heap -p -a 0af78ff0
    address 0af78ff0 found in
    _DPH_HEAP_ROOT @ 321000
    in busy allocation (  DPH_HEAP_BLOCK:  UserAddr  UserSize - VirtAddr  VirtSize)
                                 af80068:   af78ff0         c -  af78000      2000
    66248e89 verifier!AVrfDebugPageHeapAllocate+0x00000229
    7760103e ntdll!RtlDebugAllocateHeap+0x00000030
    775babe2 ntdll!RtlpAllocateHeap+0x000000c4
    775634a1 ntdll!RtlAllocateHeap+0x0000023a
    01c521e5 pdfium_test!_malloc_base+0x00000038 [minkernel\crts\ucrt\src\appcrt\heap\malloc_base.cpp @ 34]
    01c2dbea pdfium_test!operator new+0x0000001a [f:\dd\vctools\crt\vcstartup\src\heap\new_scalar.cpp @ 34]
    01b187c8 pdfium_test!CXFA_LayoutPageMgr::CXFA_LayoutPageMgr+0x00000028 [C:\pdfium\xfa\fxfa\parser\cxfa_layoutpagemgr.cpp @ 275]
    01b0f9d4 pdfium_test!CXFA_LayoutProcessor::StartLayout+0x00000086 [C:\pdfium\xfa\fxfa\parser\cxfa_layoutprocessor.cpp @ 49]
    01aed438 pdfium_test!CXFA_FFDocView::StartLayout+0x0000003c [C:\pdfium\xfa\fxfa\cxfa_ffdocview.cpp @ 74]
    01ae5057 pdfium_test!CPDFXFA_Context::LoadXFADoc+0x000000a7 [C:\pdfium\fpdfsdk\fpdfxfa\cpdfxfa_context.cpp @ 112]
    018943ee pdfium_test!FPDF_LoadXFA+0x00000022 [C:\pdfium\fpdfsdk\fpdf_view.cpp @ 265]
    012f1edb pdfium_test!main+0x00000edb [C:\pdfium\samples\pdfium_test.cc @ 911]
    01c2e61b pdfium_test!__scrt_common_main_seh+0x000000f9 [f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl @ 283]
    7677343d kernel32!BaseThreadInitThunk+0x0000000e
    77569832 ntdll!__RtlUserThreadStart+0x00000070
    77569805 ntdll!_RtlUserThreadStart+0x0000001b

Here the size of the heap buffer was 12 bytes. Actually, that's ``CXFA_LayoutPageMgr::m_CurrentContainerRecordIter``。

pdfium_test.exe crashed when calling ``GetCurrentContainerRecord()`` in ``CXFA_LayoutPageMgr::GetAvailHeight()``.

```
float CXFA_LayoutPageMgr::GetAvailHeight() {
  CXFA_ContainerLayoutItem* pLayoutItem =
      GetCurrentContainerRecord()->pCurContentArea;     // ----> crashed!!!
  if (!pLayoutItem || !pLayoutItem->m_pFormNode)
    return 0.0f;

  float fAvailHeight = pLayoutItem->m_pFormNode->JSObject()
                           ->GetMeasure(XFA_Attribute::H)
                           .ToUnit(XFA_Unit::Pt);
  if (fAvailHeight >= XFA_LAYOUT_FLOAT_PERCISION)
    return fAvailHeight;
  if (m_CurrentContainerRecordIter == m_ProposedContainerRecords.begin())
    return 0.0f;
  return FLT_MAX;
}

  CXFA_ContainerRecord* /* CXFA_LayoutPageMgr:: */ GetCurrentContainerRecord() {
    return *m_CurrentContainerRecordIter;               // ----> crashed!!!
  }
```

This vulnerability is more likely exploitable since the value of ``pLayoutItem`` may be controlled by attacker.

```
  float fAvailHeight = pLayoutItem->m_pFormNode->JSObject()
                           ->GetMeasure(XFA_Attribute::H)
                           .ToUnit(XFA_Unit::Pt);
```

VERSION
Chrome Version: pdfium with XFA enabled
Operating System: Windows

REPRODUCTION CASE
A minimized proof-of-concept file will be attached.

FOR CRASHES, PLEASE INCLUDE THE FOLLOWING ADDITIONAL INFORMATION
Type of crash: [tab, browser, etc.]
Crash State: [see link above: stack trace *with symbols*, registers,
exception record]
Client ID (if relevant): [see link above]

 
Please note it only affects XFA enabled PDFium. Some of the build arguments:

```
pdf_enable_xfa = true
pdf_enable_v8 = true
is_asan=true
```
I'm sorry for forgetting add PDFium in title. Please help add the following label,
thank you very much.

Components: Internals>Plugins>PDF
Project Member

Comment 3 by ClusterFuzz, May 13 2018

ClusterFuzz is analyzing your testcase. Developers can follow the progress at https://clusterfuzz.com/testcase?key=4899555070705664.
Project Member

Comment 4 by ClusterFuzz, May 13 2018

ClusterFuzz is analyzing your testcase. Developers can follow the progress at https://clusterfuzz.com/testcase?key=4826538613407744.

Comment 5 by rsesek@chromium.org, May 13 2018

Components: Internals>Plugins>PDF
Labels: Security_Severity-Medium Security_Impact-None Pri-2
Owner: dsinclair@chromium.org
Status: Assigned (was: Unconfirmed)
Blocking: 62400
Owner: hnakashima@chromium.org
Labels: OS-Chrome OS-Linux OS-Mac OS-Windows
Status: Started (was: Assigned)
Cc: hnakashima@chromium.org dsinclair@chromium.org
Owner: tsepez@chromium.org
Tom, Dan, I need some input here. The root of this bug is this block in the POC: 

    <subform layout="tb">
        <event activity="initialize">
            <submit/>
        </event>
    </subform>

"initialize" triggers when the document is loaded, and a submit is immediately performed. Putting aside the security considerations of the uninitialized memory access for a moment, I don't think that's a thing we want to ever do. Do we even want to support Submit at all?
Cc: -dsinclair@chromium.org tsepez@chromium.org
Owner: dsinclair@chromium.org
No idea, Dan?
At least for MVP, I think we can put Submit off the table. I'd hope the initialize would fire after the subform is initialized? So, hopefully we can fixup anything uninitialized?
It triggers during the initialization. The relevant call tree:
 
CXFA_FFDocView::StartLayout() begins
  CXFA_LayoutProcessor::StartLayout() begins
    -> CXFA_LayoutPageMgr::AppendNewPage() fills m_ProposedContainerRecords
  CXFA_LayoutProcessor::StartLayout() ends
  CXFA_FFDocView::InitLayout() begins
    CPDFXFA_DocEnvironment::OnBeforeNotifySubmit() begins
      CXFA_LayoutProcessor::StartLayout() begins
        -> CXFA_LayoutPageMgr::ClearData() clears m_ProposedContainerRecords
        -> CXFA_LayoutPageMgr::AppendNewPage() fills m_ProposedContainerRecords
      CXFA_LayoutProcessor::StartLayout() ends
      CXFA_LayoutProcessor::DoLayout() begins
        -> CXFA_LayoutPageMgr::GetAvailHeight() accesses m_ProposedContainerRecords
        -> CXFA_LayoutPageMgr::ClearData() clears m_ProposedContainerRecords
      CXFA_LayoutProcessor::DoLayout() ends
    CPDFXFA_DocEnvironment::OnBeforeNotifySubmit() ends
  CXFA_FFDocView::InitLayout() ends
CXFA_FFDocView::StartLayout() ends
CXFA_LayoutProcessor::DoLayout() begins
  -> CXFA_LayoutPageMgr::GetAvailHeight() accesses m_ProposedContainerRecords, crashes

The submit operation inside CXFA_FFDocView::StartLayout() causes m_ProposedContainerRecords to be cleared, while there is an assumption that it would be valid after CXFA_FFDocView::StartLayout() returns.
Cc: dsinclair@chromium.org
Owner: hnakashima@chromium.org
Status: Fixed (was: Started)
Project Member

Comment 16 by bugdroid1@chromium.org, Jul 25

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/033de4c4fc396ff11ae80766cf7d00d86f4afc62

commit 033de4c4fc396ff11ae80766cf7d00d86f4afc62
Author: pdfium-chromium-autoroll <pdfium-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Wed Jul 25 23:20:07 2018

Roll src/third_party/pdfium 1f7db295b1de..a5d2bf1131fe (8 commits)

https://pdfium.googlesource.com/pdfium.git/+log/1f7db295b1de..a5d2bf1131fe


git log 1f7db295b1de..a5d2bf1131fe --date=short --no-merges --format='%ad %ae %s'
2018-07-25 thestig@chromium.org Remove CFX_MemoryStream uses in tests.
2018-07-25 tsepez@chromium.org Use struct {Single,Range}Cmap in FPDFAPI_CIDFromCharCode().
2018-07-25 hinoka@google.com README.md: Update waterfall location
2018-07-25 thestig@chromium.org Change CFX_BufferSeekableReadStream to take a span.
2018-07-25 thestig@chromium.org Only build cfx_fileaccess_windows.cpp on Windows.
2018-07-25 thestig@chromium.org Move CPDF_SyntaxParser init methods into ctor.
2018-07-25 hnakashima@chromium.org Disable submit in XFA forms.
2018-07-25 tsepez@chromium.org Introduce ToXMLElement() checked downcast helper function


Created with:
  gclient setdep -r src/third_party/pdfium@a5d2bf1131fe

The AutoRoll server is located here: https://pdfium-roll.skia.org

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.



BUG= chromium:860896 , chromium:842503 
TBR=dsinclair@chromium.org

Change-Id: If7bcdd2cc19be54976a33cb521e5fe2717fae483
Reviewed-on: https://chromium-review.googlesource.com/1150362
Reviewed-by: pdfium-chromium-autoroll <pdfium-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Commit-Queue: pdfium-chromium-autoroll <pdfium-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#578120}
[modify] https://crrev.com/033de4c4fc396ff11ae80766cf7d00d86f4afc62/DEPS

Project Member

Comment 17 by sheriffbot@chromium.org, Jul 26

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Labels: reward-topanel
hnakashima@ - thanks for the fix. Might it be worth adding a test case for this in case submit is re-enabled in the future without the root cause having been addressed?
Labels: -reward-topanel reward-unpaid reward-3000
*** Boilerplate reminders! ***
Please do NOT publicly disclose details until a fix has been released to all our users. Early public disclosure may cancel the provisional reward. Also, please be considerate about disclosure when the bug affects a core library that may be used by other products. Please do NOT share this information with third parties who are not directly involved in fixing the bug. Doing so may cancel the provisional reward. Please be honest if you have already disclosed anything publicly or to third parties. Lastly, we understand that some of you are not interested in money. We offer the option to donate your reward to an eligible charity. If you prefer this option, let us know and we will also match your donation - subject to our discretion. Any rewards that are unclaimed after 12 months will be donated to a charity of our choosing.
*********************************
And $3,000 for this one :-)
Labels: M-70
Labels: -reward-unpaid reward-inprocess
Project Member

Comment 24 by sheriffbot@chromium.org, Nov 1

Labels: -Restrict-View-SecurityNotify allpublic
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

Sign in to add a comment