New issue
Advanced search Search tips

Issue 807214 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac , Fuchsia
Pri: 1
Type: Bug-Security

Blocking:
issue 62400



Sign in to add a comment

Security: global-buffer-overflow in CFX_GetCSSPropertyByName

Reported by zhouzhen...@gmail.com, Jan 30 2018

Issue description

VULNERABILITY DETAILS
This issue was found by fuzzing against a 64-bit asan linux build of pdfium_test with XFA enabled.

VERSION
Operating System: Fedora 27 x86_64

REPRODUCTION CASE

Get the latest chromium source code, build pdfium_test with the following args.gn (command: ninja -C out/default pdfium_test)

---------------------
enable_nacl=false
is_debug=false
is_asan=true
pdf_use_skia = false
pdf_use_skia_paths = false
pdf_enable_xfa = true
pdf_enable_v8 = true
pdf_enable_xfa_bmp = true
pdf_enable_xfa_gif = true
pdf_enable_xfa_png = true
pdf_enable_xfa_tiff = true
symbol_level=2
----------------------

./pdfium_test /tmp/pdf_crashes/global-buffer-overflow-poc


Rendering PDF file /tmp/pdf_crashes/global-buffer-overflow-poc.
=================================================================
==10316==ERROR: AddressSanitizer: global-buffer-overflow on address 0x000003aa70b0 at pc 0x00000353bd0e bp 0x7ffd2637fd70 sp 0x7ffd2637fd68
READ of size 4 at 0x000003aa70b0 thread T0
    #0 0x353bd0d in CFX_GetCSSPropertyByName(fxcrt::StringViewTemplate<wchar_t> const&) third_party/pdfium/core/fxcrt/css/cfx_cssdatatable.cpp:133:39
    #1 0x354f152 in CFX_CSSStyleSelector::AppendInlineStyle(CFX_CSSDeclaration*, fxcrt::WideString const&) third_party/pdfium/core/fxcrt/css/cfx_cssstyleselector.cpp:152:15
    #2 0x354eccc in CFX_CSSStyleSelector::ComputeStyle(std::__1::vector<CFX_CSSDeclaration const*, std::__1::allocator<CFX_CSSDeclaration const*> > const&, fxcrt::WideString const&, fxcrt::WideString const&, CFX_CSSComputedStyle*) third_party/pdfium/core/fxcrt/css/cfx_cssstyleselector.cpp:91:7
    #3 0x35361f1 in CXFA_TextParser::ParseRichText(CFX_XMLNode*, CFX_CSSComputedStyle*) third_party/pdfium/xfa/fxfa/cxfa_textparser.cpp:235:20
    #4 0x353674d in CXFA_TextParser::ParseRichText(CFX_XMLNode*, CFX_CSSComputedStyle*) third_party/pdfium/xfa/fxfa/cxfa_textparser.cpp:251:5
    #5 0x3535c76 in CXFA_TextParser::DoParse(CFX_XMLNode*, CXFA_TextProvider*) third_party/pdfium/xfa/fxfa/cxfa_textparser.cpp:214:3
    #6 0x351bddb in CXFA_TextLayout::Loader(float, float&, bool) third_party/pdfium/xfa/fxfa/cxfa_textlayout.cpp:639:22
    #7 0x35193c4 in CXFA_TextLayout::CalcSize(CFX_STemplate<float> const&, CFX_STemplate<float> const&) third_party/pdfium/xfa/fxfa/cxfa_textlayout.cpp:392:3
    #8 0x3519a15 in CXFA_TextLayout::StartLayout(float) third_party/pdfium/xfa/fxfa/cxfa_textlayout.cpp:294:21
    #9 0x362a2f9 in CXFA_Node::StartTextLayout(CXFA_FFDoc*, float&, float&) third_party/pdfium/xfa/fxfa/parser/cxfa_node.cpp:3449:63
    #10 0x362965c in CXFA_Node::StartWidgetLayout(CXFA_FFDoc*, float&, float&) third_party/pdfium/xfa/fxfa/parser/cxfa_node.cpp:3080:5
    #11 0x35e4081 in CXFA_ItemLayoutProcessor::DoLayoutField() third_party/pdfium/xfa/fxfa/parser/cxfa_itemlayoutprocessor.cpp:2757:12
    #12 0x35cc6ff in CXFA_ItemLayoutProcessor::DoLayout(bool, float, float, CXFA_LayoutContext*) third_party/pdfium/xfa/fxfa/parser/cxfa_itemlayoutprocessor.cpp:2802:7
    #13 0x35cdaf5 in CXFA_ItemLayoutProcessor::DoLayoutPositionedContainer(CXFA_LayoutContext*) third_party/pdfium/xfa/fxfa/parser/cxfa_itemlayoutprocessor.cpp:1711:17
    #14 0x35cc7ff in CXFA_ItemLayoutProcessor::DoLayout(bool, float, float, CXFA_LayoutContext*) third_party/pdfium/xfa/fxfa/parser/cxfa_itemlayoutprocessor.cpp:2791:11
    #15 0x35cdaf5 in CXFA_ItemLayoutProcessor::DoLayoutPositionedContainer(CXFA_LayoutContext*) third_party/pdfium/xfa/fxfa/parser/cxfa_itemlayoutprocessor.cpp:1711:17
    #16 0x35cc7ff in CXFA_ItemLayoutProcessor::DoLayout(bool, float, float, CXFA_LayoutContext*) third_party/pdfium/xfa/fxfa/parser/cxfa_itemlayoutprocessor.cpp:2791:11
    #17 0x35ddd32 in (anonymous namespace)::InsertFlowedItem(CXFA_ItemLayoutProcessor*, CXFA_ItemLayoutProcessor*, bool, bool, float, XFA_AttributeEnum, unsigned char*, std::__1::vector<CXFA_ContentLayoutItem*, std::__1::allocator<CXFA_ContentLayoutItem*> > (&) [3], bool, float, float, float, float*, float*, float*, bool*, bool*, CXFA_LayoutContext*, bool) third_party/pdfium/xfa/fxfa/parser/cxfa_itemlayoutprocessor.cpp:715:29
    #18 0x35d9fa1 in CXFA_ItemLayoutProcessor::DoLayoutFlowedContainer(bool, XFA_AttributeEnum, float, float, CXFA_LayoutContext*, bool) third_party/pdfium/xfa/fxfa/parser/cxfa_itemlayoutprocessor.cpp:2510:46
    #19 0x35cc8b5 in CXFA_ItemLayoutProcessor::DoLayout(bool, float, float, CXFA_LayoutContext*) third_party/pdfium/xfa/fxfa/parser/cxfa_itemlayoutprocessor.cpp:2784:18
    #20 0x35e5e9a in CXFA_LayoutProcessor::DoLayout() third_party/pdfium/xfa/fxfa/parser/cxfa_layoutprocessor.cpp:74:43
    #21 0x32c8224 in CXFA_FFDocView::DoLayout() third_party/pdfium/xfa/fxfa/cxfa_ffdocview.cpp:94:30
    #22 0x31cfffe in CPDFXFA_Context::LoadXFADoc() third_party/pdfium/fpdfsdk/fpdfxfa/cpdfxfa_context.cpp:121:18
    #23 0x270393d in FPDF_LoadXFA third_party/pdfium/fpdfsdk/fpdfview.cpp:599:63
    #24 0xbd7725 in (anonymous namespace)::RenderPdf(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, char const*, unsigned long, (anonymous namespace)::Options const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) third_party/pdfium/samples/pdfium_test.cc:1435:10
    #25 0xbd4ec8 in main third_party/pdfium/samples/pdfium_test.cc:1630:5
    #26 0x7f3c3bb35009 in __libc_start_main (/lib64/libc.so.6+0x21009)

0x000003aa70b0 is located 16 bytes to the right of global variable 'g_CFX_CSSProperties' defined in '../../third_party/pdfium/core/fxcrt/css/cfx_cssdatatable.cpp:16:35' (0x3aa6ce0) of size 960
SUMMARY: AddressSanitizer: global-buffer-overflow third_party/pdfium/core/fxcrt/css/cfx_cssdatatable.cpp:133:39 in CFX_GetCSSPropertyByName(fxcrt::StringViewTemplate<wchar_t> const&)
Shadow bytes around the buggy address:
  0x00008074cdc0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x00008074cdd0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x00008074cde0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x00008074cdf0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x00008074ce00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x00008074ce10: 00 00 00 00 f9 f9[f9]f9 f9 f9 f9 f9 f9 f9 f9 f9
  0x00008074ce20: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9
  0x00008074ce30: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x00008074ce40: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x00008074ce50: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x00008074ce60: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==10316==ABORTING

Testcase is in the attachment.

 
Components: Internals>Plugins>PDF
Blocking: 62400
Labels: Pri-1
Owner: rharrison@chromium.org
Status: Started (was: Unconfirmed)
I have been able to reproduce this locally

Comment 5 by palmer@chromium.org, Jan 30 2018

Cc: tsepez@chromium.org dsinclair@chromium.org
Labels: M-65 Security_Severity-High Security_Impact-None OS-Android OS-Chrome OS-Fuchsia OS-Linux OS-Mac OS-Windows
Since we don't ship XFA yet, I wonder if I should mark this Security_Impact-None? tsepez, what do you think?
Labels: -M-65
That's what we usually do, remove the milestone, set impact to None and leave Severity alone.
Project Member

Comment 7 by bugdroid1@chromium.org, Jan 30 2018

The following revision refers to this bug:
  https://pdfium.googlesource.com/pdfium/+/802eaea7696e2e1aa8d6d76d1fee39fbe1c7794b

commit 802eaea7696e2e1aa8d6d76d1fee39fbe1c7794b
Author: Ryan Harrison <rharrison@chromium.org>
Date: Tue Jan 30 20:24:50 2018

Clean up CSS Data Table entries and access

This cleans up the entries in the table to no longer have a marker for
size, and also removes a hand rolled search. This prevents an out of
bounds issue that had been reported and addresses another potential
out of bounds issue.

BUG= chromium:807214 

Change-Id: I3d3ab5a3a174dd4dcec56fa7ee7a0e6c2805bfaa
Reviewed-on: https://pdfium-review.googlesource.com/24690
Reviewed-by: Ryan Harrison <rharrison@chromium.org>
Reviewed-by: dsinclair <dsinclair@chromium.org>
Commit-Queue: dsinclair <dsinclair@chromium.org>
Commit-Queue: Ryan Harrison <rharrison@chromium.org>

[modify] https://crrev.com/802eaea7696e2e1aa8d6d76d1fee39fbe1c7794b/core/fxcrt/css/cfx_css.h
[modify] https://crrev.com/802eaea7696e2e1aa8d6d76d1fee39fbe1c7794b/core/fxcrt/css/cfx_cssdatatable.h
[modify] https://crrev.com/802eaea7696e2e1aa8d6d76d1fee39fbe1c7794b/core/fxcrt/css/cfx_cssdatatable.cpp

Status: Fixed (was: Started)
Project Member

Comment 9 by bugdroid1@chromium.org, Jan 31 2018

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

commit 18f244635e321ec2e54862186f5727cb29ce8309
Author: pdfium-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com <pdfium-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Wed Jan 31 01:02:34 2018

Roll src/third_party/pdfium/ 1917cdd8c..233466005 (15 commits)

https://pdfium.googlesource.com/pdfium.git/+log/1917cdd8c90b..2334660053e0

$ git log 1917cdd8c..233466005 --date=short --no-merges --format='%ad %ae %s'
2018-01-30 npm Use unsigned for char width
2018-01-30 dsinclair Shuffle more code out of CXFA_Node
2018-01-30 dsinclair Move CheckButton code from CXFA_Node to CXFA_CheckButton
2018-01-30 rharrison Clean up CSS Data Table entries and access
2018-01-30 tsepez Remove bare new from JS_Define.h
2018-01-30 hnakashima Check if opj_image_data_alloc returned null.
2018-01-30 dsinclair Cleanup some param passing code
2018-01-30 hnakashima Guard usages of tellp(). It may return -1 in error cases.
2018-01-30 dsinclair Cleanup duplicate RunScript code
2018-01-30 thestig Remove not reachable branch in fxge code.
2018-01-30 thestig Use anonymous namespace in gdiplus code.
2018-01-30 dsinclair Cleanup some SDK code
2018-01-30 tsepez Revert "Revert "Use UnownedPtr instead of T* in MaybeOwned.""
2018-01-30 tsepez Revert "Use UnownedPtr instead of T* in MaybeOwned."
2018-01-30 tsepez Use UnownedPtr instead of T* in MaybeOwned.

Created with:
  roll-dep src/third_party/pdfium
BUG= 807214 ,805881


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.


TBR=dsinclair@chromium.org

Change-Id: Iccd05c3211b080fc7392a482d0c2be722c1ec683
Reviewed-on: https://chromium-review.googlesource.com/894482
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@{#533128}
[modify] https://crrev.com/18f244635e321ec2e54862186f5727cb29ce8309/DEPS

Project Member

Comment 10 by sheriffbot@chromium.org, Feb 8 2018

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Project Member

Comment 11 by sheriffbot@chromium.org, May 9 2018

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
Labels: reward-topanel
Labels: -reward-topanel reward-unpaid reward-1000
*** 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.
*********************************
Labels: -Security_Severity-High Security_Severity-Medium
Hi zhouzhenster@, the VRP panel chose to track this as medium severity, and award $1,000 for the report - thanks!
Labels: -reward-unpaid reward-inprocess

Sign in to add a comment