New issue
Advanced search Search tips

Issue 856354 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

Security: [pdfium] CJS_Field::m_pJSDoc may outlive the document.

Project Member Reported by tsepez@chromium.org, Jun 25 2018

Issue description

Making the field an unowned ptr rapidly gives:

READ of size 1 at 0x60b00001d050 thread T0
    #0 0x2dae393 in ProbeForLowSeverityLifetimeIssue ./../../core/fxcrt/unowned_ptr.h:110:7
    #1 0x2dae393 in ~UnownedPtr ./../../core/fxcrt/unowned_ptr.h:60:0
    #2 0x2dae393 in ~CJS_Field ./../../fxjs/cjs_field.cpp:177:0
    #3 0x2dae393 in CJS_Field::~CJS_Field() ./../../fxjs/cjs_field.cpp:177:0
    #4 0x2e28233 in JSDestructor(v8::Local<v8::Object>) ./../../fxjs/js_define.cpp:171:3
    #5 0x2d5123e in operator() ./../../buildtools/third_party/libc++/trunk/include/functional:1913:12
    #6 0x2d5123e in V8TemplateMapTraits::Dispose(v8::Isolate*, v8::Global<v8::Object>, CFXJS_PerObjectData*) ./../../fxjs/cfxjs_engine.cpp:231:0
    #7 0x2d594bc in v8::PersistentValueMapBase<CFXJS_PerObjectData*, v8::Object, V8TemplateMapTraits>::Clear() ./../../v8/include/v8

0x60b00001d050 is located 0 bytes inside of 104-byte region [0x60b00001d050,0x60b00001d0b8)
freed by thread T0 here:
    #0 0xbebb82 in operator delete(void*) _asan_rtl_:3
    #1 0x2e28233 in JSDestructor(v8::Local<v8::Object>) ./../../fxjs/js_define.cpp:171:3
    #2 0x2d57053 in operator() ./../../buildtools/third_party/libc++/trunk/include/functional:1913:12
    #3 0x2d57053 in CFXJS_Engine::ReleaseEngine() ./../../fxjs/cfxjs_engine.cpp:501:0
    #4 0x2d5df61 in CJS_Runtime::~CJS_Runtime() ./../../fxjs/cjs_runtime.cpp:83:3
 
 

Comment 1 by tsepez@chromium.org, Jun 25 2018

So we have an object that may live until V8 decides to GC.  Unfortunately, we may have been unloaded by then.  May need to observe document destruction.

Comment 2 by tsepez@chromium.org, Jun 25 2018

Components: Internals>Plugins>PDF
Labels: Security_Severity-Low Security_Impact-Stable
Owner: tsepez@chromium.org
Status: Started (was: Unconfirmed)

Comment 3 by tsepez@chromium.org, Jun 25 2018

Cc: thestig@chromium.org dsinclair@chromium.org
Project Member

Comment 4 by sheriffbot@chromium.org, Jun 26 2018

Labels: Pri-2

Comment 6 by tsepez@chromium.org, Jun 26 2018

Status: Fixed (was: Started)
Project Member

Comment 7 by bugdroid1@chromium.org, Jun 26 2018

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

commit a856e3247c018176370ec0bfd62a8add1b141ba5
Author: pdfium-chromium-autoroll <pdfium-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Tue Jun 26 20:16:18 2018

Roll src/third_party/pdfium 75ee53784b5e..0a6dbeffbc61 (11 commits)

https://pdfium.googlesource.com/pdfium.git/+log/75ee53784b5e..0a6dbeffbc61


git log 75ee53784b5e..0a6dbeffbc61 --date=short --no-merges --format='%ad %ae %s'
2018-06-26 tsepez@chromium.org Add some more consts to unowned pointers.
2018-06-26 tsepez@chromium.org Use pdfium::span in CPDF_CID2UnicodeMap.
2018-06-26 hnakashima@chromium.org Fix Integer-overflow in CFX_RTFBreak::AppendChar_Tab
2018-06-26 art-snake@yandex-team.ru Simplify CPDF_Parser::RebuildCrossRef.
2018-06-26 art-snake@yandex-team.ru Unify CPDF_Document loading methods.
2018-06-26 art-snake@yandex-team.ru Add test which verify, that "Info" from linearized doc is correctly saved.
2018-06-26 art-snake@yandex-team.ru Implement CPDF_ObjStream.
2018-06-26 hnakashima@chromium.org Write pixel .evt test for dynamic_list_box_allow_multiple_selection.
2018-06-26 tsepez@chromium.org Make CPDF_Document::m_pRootDict an UnownedPtr<>.
2018-06-26 vmiklos@collabora.co.uk Add FPDFTextObj_GetFontSize() API
2018-06-26 tsepez@chromium.org Use ObservedPtr for CJS_Field::m_pJSField


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

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:844367 , chromium:856354 
TBR=dsinclair@chromium.org

Change-Id: I53417b981430471c1a1fbb026c96004f86483522
Reviewed-on: https://chromium-review.googlesource.com/1115405
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@{#570511}
[modify] https://crrev.com/a856e3247c018176370ec0bfd62a8add1b141ba5/DEPS

Project Member

Comment 8 by sheriffbot@chromium.org, Jun 27 2018

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Labels: OS-Chrome OS-Linux OS-Windows
Project Member

Comment 10 by sheriffbot@chromium.org, Oct 3

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