New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 862059 link

Starred by 2 users

Issue metadata

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



Sign in to add a comment

Security: Bad cast in JSPropGetter in js_define.h

Reported by chamal.d...@gmail.com, Jul 10

Issue description

VULNERABILITY DETAILS

This bug is present in JSPropSetter method in fxjs/js_define.h file.
I think JSMethod and JSPropGetter methods in fxjs/js_define.h also has the same bug.
But I could not prepare a test case for those methods.

Below is a code section from JSPropSetter method.

template <class C, CJS_Return (C::*M)(CJS_Runtime*)>
void JSPropGetter(const char* prop_name_string,
                  const char* class_name_string,
                  v8::Local<v8::String> property,
                  const v8::PropertyCallbackInfo<v8::Value>& info) {
  CJS_Object* pJSObj = CFXJS_Engine::GetObjectPrivate(info.Holder());
  .....
  C* pObj = static_cast<C*>(pJSObj);
  CJS_Return result = (pObj->*M)(pRuntime);
  ......
}

Above method uses CFXJS_Engine::GetObjectPrivate method to get private CJS_Object from javascript object.
Then try to cast CJS_Object to the subclass type of "C"(see template above method signature).
But does not check wether the actual type of CJS_Object matches type "C".

This is the code of CFXJS_Engine::GetObjectPrivate method.

CJS_Object* CFXJS_Engine::GetObjectPrivate(v8::Local<v8::Object> pObj) {
  CFXJS_PerObjectData* pData = CFXJS_PerObjectData::GetFromObject(pObj);
  if (!pData && !pObj.IsEmpty()) {
    // It could be a global proxy object.

    v8::Local<v8::Value> v = pObj->GetPrototype(); 
     /* Note:  Prototype can be changed with  Object.setPrototypeOf */

    if (v->IsObject()) {
      pData = CFXJS_PerObjectData::GetFromObject(
          v->ToObject(v8::Isolate::GetCurrent()->GetCurrentContext())
              .ToLocalChecked());
    }
  }
  return pData ? pData->m_pPrivate.get() : nullptr;
}  

Setting the prototype of javascript object to a diffent type of object makes CFXJS_Engine::GetObjectPrivate method to return
incorrect type of CJS_Object to JSPropGetter method.
ex. a = new app.constructor;
    Object.setPrototypeOf(a,global);

VERSION
Chrome Version: [67.0.3396.99] + [stable]
                [69.0.3487.0] + [Trunk build]
Operating System: [Windows 10 , Ubuntu 16.04]

REPRODUCTION CASE
1. Open attached cast.pdf with chrome.
   PDF plugin process will crash.

FOR CRASHES, PLEASE INCLUDE THE FOLLOWING ADDITIONAL INFORMATION
Type of crash: [PDF plugin process]
Crash State: [Address Sanitizer output]

This is the address sanitizer output produced by test case. 
But it does not show the actual stacktrace for bad cast bug.

==1==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x608000019701 at pc 0x55d70318592e bp 0x7ffcf4828700 sp 0x7ffcf48286f8
READ of size 8 at 0x608000019701 thread T0 (chrome)
    #0 0x55d70318592d in __insert_node_at /chromium/src/out/asan/../../buildtools/third_party/libc++/trunk/include/__tree:2112:25
    #1 0x55d70318592d in std::__1::pair<std::__1::__tree_iterator<std::__1::__value_type<fxcrt::ByteString, std::__1::unique_ptr<CJS_Global::JSGlobalData, std::__1::default_delete<CJS_Global::JSGlobalData> > >, std::__1::__tree_node<std::__1::__value_type<fxcrt::ByteString, std::__1::unique_ptr<CJS_Global::JSGlobalData, std::__1::default_delete<CJS_Global::JSGlobalData> > >, void*>*, long>, bool> std::__1::__tree<std::__1::__value_type<fxcrt::ByteString, std::__1::unique_ptr<CJS_Global::JSGlobalData, std::__1::default_delete<CJS_Global::JSGlobalData> > >, std::__1::__map_value_compare<fxcrt::ByteString, std::__1::__value_type<fxcrt::ByteString, std::__1::unique_ptr<CJS_Global::JSGlobalData, std::__1::default_delete<CJS_Global::JSGlobalData> > >, std::__1::less<fxcrt::ByteString>, true>, std::__1::allocator<std::__1::__value_type<fxcrt::ByteString, std::__1::unique_ptr<CJS_Global::JSGlobalData, std::__1::default_delete<CJS_Global::JSGlobalData> > > > >::__emplace_unique_key_args<fxcrt::ByteString, std::__1::piecewise_construct_t const&, std::__1::tuple<fxcrt::ByteString const&>, std::__1::tuple<> >(fxcrt::ByteString const&, std::__1::piecewise_construct_t const&&&, std::__1::tuple<fxcrt::ByteString const&>&&, std::__1::tuple<>&&) /chromium/src/out/asan/../../buildtools/third_party/libc++/trunk/include/__tree:2141:0
    #2 0x55d7031843da in operator[] /chromium/src/out/asan/../../buildtools/third_party/libc++/trunk/include/map:1319:20
    #3 0x55d7031843da in CJS_Global::SetGlobalVariables(fxcrt::ByteString const&, JS_GlobalDataType, double, bool, fxcrt::ByteString const&, v8::Local<v8::Object>, bool) /chromium/src/out/asan/../../third_party/pdfium/fxjs/cjs_global.cpp:563:0
    #4 0x55d703183a99 in CJS_Global::SetProperty(CJS_Runtime*, wchar_t const*, v8::Local<v8::Value>) /chromium/src/out/asan/../../third_party/pdfium/fxjs/cjs_global.cpp:278:12
    #5 0x55d703180b89 in JSSpecialPropPut<CJS_Global> /chromium/src/out/asan/../../third_party/pdfium/fxjs/cjs_global.cpp:86:50
    #6 0x55d703180b89 in CJS_Global::putprop_static(v8::Local<v8::Name>, v8::Local<v8::Value>, v8::PropertyCallbackInfo<v8::Value> const&) /chromium/src/out/asan/../../third_party/pdfium/fxjs/cjs_global.cpp:170:0
    #7 0x55d6f1ae97cf in v8::internal::PropertyCallbackArguments::CallNamedSetter(v8::internal::Handle<v8::internal::InterceptorInfo>, v8::internal::Handle<v8::internal::Name>, v8::internal::Handle<v8::internal::Object>) /chromium/src/out/asan/../../v8/src/api-arguments-inl.h:168:3
    #8 0x55d6f1cc54c0 in v8::internal::(anonymous namespace)::SetPropertyWithInterceptorInternal(v8::internal::LookupIterator*, v8::internal::Handle<v8::internal::InterceptorInfo>, v8::internal::ShouldThrow, v8::internal::Handle<v8::internal::Object>) /chromium/src/out/asan/../../v8/src/objects.cc:1845:20
    #9 0x55d6f1cf2bd6 in SetPropertyWithInterceptor /chromium/src/out/asan/../../v8/src/objects.cc:4884:10
    #10 0x55d6f1cf2bd6 in v8::internal::Object::SetPropertyInternal(v8::internal::LookupIterator*, v8::internal::Handle<v8::internal::Object>, v8::internal::LanguageMode, v8::internal::Object::StoreFromKeyed, bool*) /chromium/src/out/asan/../../v8/src/objects.cc:4931:0
    #11 0x55d6f1cf279c in v8::internal::Object::SetProperty(v8::internal::LookupIterator*, v8::internal::Handle<v8::internal::Object>, v8::internal::LanguageMode, v8::internal::Object::StoreFromKeyed) /chromium/src/out/asan/../../v8/src/objects.cc:5014:9
    #12 0x55d6f1ac166a in v8::internal::StoreIC::Store(v8::internal::Handle<v8::internal::Object>, v8::internal::Handle<v8::internal::Name>, v8::internal::Handle<v8::internal::Object>, v8::internal::Object::StoreFromKeyed) /chromium/src/out/asan/../../v8/src/ic/ic.cc:1429:3
    #13 0x55d6f1ad319c in __RT_impl_Runtime_StoreIC_Miss /chromium/src/out/asan/../../v8/src/ic/ic.cc:2283:5
    #14 0x55d6f1ad319c in v8::internal::Runtime_StoreIC_Miss(int, v8::internal::Object**, v8::internal::Isolate*) /chromium/src/out/asan/../../v8/src/ic/ic.cc:2269:0
    #15 0x55d6f27b49cd in v8_Default_embedded_blob_ embedded.cc:?
    #16 0x55d6f27b49cd in ?? ??:0
    #11 0x7ec7f408675b  (<unknown module>)
    #12 0x7ec7f40086a5  (<unknown module>)
    #17 0x55d6f2728502 in v8_Default_embedded_blob_ embedded.cc:?
    #18 0x55d6f2728502 in ?? ??:0
    #14 0x7ec7f40041c0  (<unknown module>)
    #19 0x55d6f188e146 in Call /chromium/src/out/asan/../../v8/src/simulator.h:113:12
    #20 0x55d6f188e146 in v8::internal::(anonymous namespace)::Invoke(v8::internal::Isolate*, bool, v8::internal::Handle<v8::internal::Object>, v8::internal::Handle<v8::internal::Object>, int, v8::internal::Handle<v8::internal::Object>*, v8::internal::Handle<v8::internal::Object>, v8::internal::Execution::MessageHandling, v8::internal::Execution::Target) /chromium/src/out/asan/../../v8/src/execution.cc:155:0
    #21 0x55d6f188d993 in CallInternal /chromium/src/out/asan/../../v8/src/execution.cc:191:10
    #22 0x55d6f188d993 in v8::internal::Execution::Call(v8::internal::Isolate*, v8::internal::Handle<v8::internal::Object>, v8::internal::Handle<v8::internal::Object>, int, v8::internal::Handle<v8::internal::Object>*) /chromium/src/out/asan/../../v8/src/execution.cc:202:0
    #23 0x55d6f0ea8f9c in v8::Script::Run(v8::Local<v8::Context>) /chromium/src/out/asan/../../v8/src/api.cc:2209:7
    #24 0x55d7031752cd in CFXJS_Engine::Execute(fxcrt::WideString const&) /chromium/src/out/asan/../../third_party/pdfium/fxjs/cfxjs_engine.cpp:532:25
    #25 0x55d70317c221 in CJS_Runtime::ExecuteScript(fxcrt::WideString const&) /chromium/src/out/asan/../../third_party/pdfium/fxjs/cjs_runtime.cpp:182:10
    #26 0x55d70320fcc7 in CJS_EventContext::RunScript(fxcrt::WideString const&) /chromium/src/out/asan/../../third_party/pdfium/fxjs/cjs_event_context.cpp:53:23
    #27 0x55d702c7c8d6 in RunScript /chromium/src/out/asan/../../third_party/pdfium/fpdfsdk/cpdfsdk_actionhandler.cpp:546:13
    #28 0x55d702c7c8d6 in CPDFSDK_ActionHandler::RunDocumentOpenJavaScript(CPDFSDK_FormFillEnvironment*, fxcrt::WideString const&, fxcrt::WideString const&) /chromium/src/out/asan/../../third_party/pdfium/fpdfsdk/cpdfsdk_actionhandler.cpp:455:0
    #29 0x55d702c7c58a in CPDFSDK_ActionHandler::DoAction_JavaScript(CPDF_Action const&, fxcrt::WideString, CPDFSDK_FormFillEnvironment*) /chromium/src/out/asan/../../third_party/pdfium/fpdfsdk/cpdfsdk_actionhandler.cpp:37:7
    #30 0x55d702c853ae in CPDFSDK_FormFillEnvironment::ProcJavascriptFun() /chromium/src/out/asan/../../third_party/pdfium/fpdfsdk/cpdfsdk_formfillenvironment.cpp:563:25
    #31 0x55d702bea77c in chrome_pdf::PDFiumEngine::FinishLoadingDocument() /chromium/src/out/asan/../../pdf/pdfium/pdfium_engine.cc:1031:3
    #32 0x55d702c01d8c in chrome_pdf::PDFiumEngine::ContinueLoadingDocument(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) /chromium/src/out/asan/../../pdf/pdfium/pdfium_engine.cc:2619:5
    #33 0x55d702be912b in chrome_pdf::PDFiumEngine::LoadDocument() /chromium/src/out/asan/../../pdf/pdfium/pdfium_engine.cc:2532:5
    #34 0x55d702c11bec in chrome_pdf::DocumentLoaderImpl::DidRead(int) /chromium/src/out/asan/../../pdf/document_loader_impl.cc:0:7
    #35 0x55d702c12acb in operator() /chromium/src/out/asan/../../ppapi/utility/completion_callback_factory.h:6......

0x608000019701 is located 31 bytes to the left of 96-byte region [0x608000019720,0x608000019780)
allocated by thread T0 (chrome) here:
    #0 0x55d6ec253412 in operator new(unsigned long) _asan_rtl_:3
    #1 0x55d703181740 in MakeUnique<CJS_Global, v8::Local<v8::Object> &, CJS_Runtime *> /chromium/src/out/asan/../../third_party/pdfium/third_party/base/ptr_util.h:56:29
    #2 0x55d703181740 in void JSConstructor<CJS_Global>(CFXJS_Engine*, v8::Local<v8::Object>) /chromium/src/out/asan/../../third_party/pdfium/fxjs/js_define.h:54:0
    #3 0x55d7031744e2 in operator() /chromium/src/out/asan/../../buildtools/third_party/libc++/trunk/include/functional:1913:12
    #4 0x55d7031744e2 in CFXJS_Engine::NewFXJSBoundObject(int, bool) /chromium/src/out/asan/../../third_party/pdfium/fxjs/cfxjs_engine.cpp:561:0
    #5 0x55d703173af9 in CFXJS_Engine::InitializeEngine() /chromium/src/out/asan/../../third_party/pdfium/fxjs/cfxjs_engine.cpp:468:35
    #6 0x55d70317ab0d in CJS_Runtime::CJS_Runtime(CPDFSDK_FormFillEnvironment*) /chromium/src/out/asan/../../third_party/pdfium/fxjs/cjs_runtime.cpp:77:3
    #7 0x55d70316e33d in MakeUnique<CJS_Runtime, CPDFSDK_FormFillEnvironment *&> /chromium/src/out/asan/../../third_party/pdfium/third_party/base/ptr_util.h:56:33
    #8 0x55d70316e33d in IJS_Runtime::Create(CPDFSDK_FormFillEnvironment*) /chromium/src/out/asan/../../third_party/pdfium/fxjs/ijs_runtime.cpp:35:0
    #9 0x55d702c8368e in CPDFSDK_FormFillEnvironment::GetIJSRuntime() /chromium/src/out/asan/../../third_party/pdfium/fpdfsdk/cpdfsdk_formfillenvironment.cpp:212:21
    #10 0x55d702c7c7f2 in RunScript /chromium/src/out/asan/../../third_party/pdfium/fpdfsdk/cpdfsdk_actionhandler.cpp:544:58
    #11 0x55d702c7c7f2 in CPDFSDK_ActionHandler::RunDocumentOpenJavaScript(CPDFSDK_FormFillEnvironment*, fxcrt::WideString const&, fxcrt::WideString const&) /chromium/src/out/asan/../../third_party/pdfium/fpdfsdk/cpdfsdk_actionhandler.cpp:455:0
    #12 0x55d702c7c58a in CPDFSDK_ActionHandler::DoAction_JavaScript(CPDF_Action const&, fxcrt::WideString, CPDFSDK_FormFillEnvironment*) /chromium/src/out/asan/../../third_party/pdfium/fpdfsdk/cpdfsdk_actionhandler.cpp:37:7
    #13 0x55d702c853ae in CPDFSDK_FormFillEnvironment::ProcJavascriptFun() /chromium/src/out/asan/../../third_party/pdfium/fpdfsdk/cpdfsdk_formfillenvironment.cpp:563:25
    #14 0x55d702bea77c in chrome_pdf::PDFiumEngine::FinishLoadingDocument() /chromium/src/out/asan/../../pdf/pdfium/pdfium_engine.cc:1031:3
    #15 0x55d702c01d8c in chrome_pdf::PDFiumEngine::ContinueLoadingDocument(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) /chromium/src/out/asan/../../pdf/pdfium/pdfium_engine.cc:2619:5
    #16 0x55d702be912b in chrome_pdf::PDFiumEngine::LoadDocument() /chromium/src/out/asan/../../pdf/pdfium/pdfium_engine.cc:2532:5
    #17 0x55d702c11bec in chrome_pdf::DocumentLoaderImpl::DidRead(int) /chromium/src/out/asan/../../pdf/document_loader_impl.cc:0:7
    #18 0x55d702c12acb in operator() 
.....
 
cast.pdf
2.0 KB Download
cast.pdf file contains below mentioned Javascript.

Document Javascript section
----------------------------
a = new app.constructor;
Object.setPrototypeOf(a,global);
a.calculate=true;
global.b = "b"; // This line triggers address sanitizer crash. But it is not part of the actual bug.
                // Above 3 lines causes the incorrect cast bug, but they do not trigger a crash.
Project Member

Comment 2 by ClusterFuzz, Jul 10

ClusterFuzz is analyzing your testcase. Developers can follow the progress at https://clusterfuzz.com/testcase?key=5090372544626688.
Components: Blink>JavaScript
Please set Components:Internals>Plugins>PDF. This is not a blink>javascript bug.
Components: -Blink>JavaScript Internals>Plugins>PDF
Cc: palmer@chromium.org dsinclair@chromium.org tsepez@chromium.org
Labels: OS-Chrome OS-Linux OS-Mac OS-Windows
Status: Untriaged (was: Unconfirmed)
palmer: Can you help us triage?

This can be reproduced with pdfium_test, so you don't need to build the entire browser. In a Chromium checkout, one can turn on UBSAN and get:

Rendering PDF file /tmp/cast.pdf.
../../third_party/pdfium/fxjs/js_define.h:99:13: runtime error: downcast of address 0x117837fc1a20 which does not point to an object of type 'CJS_App'
0x117837fc1a20: note: object is of type 'CJS_Global'
 00 00 00 00  38 fd f9 02 00 00 00 00  00 20 09 38 78 11 00 00  00 b1 b2 38 78 11 00 00  38 fa f9 02
              ^~~~~~~~~~~~~~~~~~~~~~~
              vptr for 'CJS_Global'
    #0 0x1e66865 in void JSPropSetter<CJS_App, &(CJS_App::set_calculate(CJS_Runtime*, v8::Local<v8::Value>))>(char const*, char const*, v8::Local<v8::String>, v8::Local<v8::Value>, v8::PropertyCallbackInfo<void> const&) third_party/pdfium/fxjs/js_define.h:99:13
    #1 0x11da77b in v8::internal::PropertyCallbackArguments::CallAccessorSetter(v8::internal::Handle<v8::internal::AccessorInfo>, v8::internal::Handle<v8::internal::Name>, v8::internal::Handle<v8::internal::Object>) v8/src/api-arguments-inl.h:297:3
   ...
Owner: thestig@chromium.org
Status: Started (was: Untriaged)
https://pdfium-review.googlesource.com/c/pdfium/+/37510
Project Member

Comment 8 by bugdroid1@chromium.org, Jul 11

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

commit ad1f7b410cd6885bd22d9ee49d9f80d3017f131f
Author: Lei Zhang <thestig@chromium.org>
Date: Wed Jul 11 13:04:43 2018

Check GetObjDefnID() in various JS functions.

Consolidate all the checks into JSGetObject(), and add GetObjDefnID()
methods for classes that are missing it.

BUG= chromium:862059 

Change-Id: I2c2b725a01dcd259ef712d2513fcf740cc410b15
Reviewed-on: https://pdfium-review.googlesource.com/37510
Commit-Queue: dsinclair <dsinclair@chromium.org>
Reviewed-by: dsinclair <dsinclair@chromium.org>

[modify] https://crrev.com/ad1f7b410cd6885bd22d9ee49d9f80d3017f131f/fxjs/cjs_app.cpp
[modify] https://crrev.com/ad1f7b410cd6885bd22d9ee49d9f80d3017f131f/fxjs/cjs_console.h
[modify] https://crrev.com/ad1f7b410cd6885bd22d9ee49d9f80d3017f131f/fxjs/js_define.h
[modify] https://crrev.com/ad1f7b410cd6885bd22d9ee49d9f80d3017f131f/fxjs/cjs_color.h
[modify] https://crrev.com/ad1f7b410cd6885bd22d9ee49d9f80d3017f131f/fxjs/cjs_console.cpp
[modify] https://crrev.com/ad1f7b410cd6885bd22d9ee49d9f80d3017f131f/fxjs/cjs_event.h
[modify] https://crrev.com/ad1f7b410cd6885bd22d9ee49d9f80d3017f131f/fxjs/cjs_app.h
[modify] https://crrev.com/ad1f7b410cd6885bd22d9ee49d9f80d3017f131f/fxjs/cjs_util.h
[modify] https://crrev.com/ad1f7b410cd6885bd22d9ee49d9f80d3017f131f/fxjs/cjs_report.cpp
[modify] https://crrev.com/ad1f7b410cd6885bd22d9ee49d9f80d3017f131f/fxjs/cjs_global.cpp
[modify] https://crrev.com/ad1f7b410cd6885bd22d9ee49d9f80d3017f131f/fxjs/cjs_global.h
[modify] https://crrev.com/ad1f7b410cd6885bd22d9ee49d9f80d3017f131f/fxjs/cjs_color.cpp
[modify] https://crrev.com/ad1f7b410cd6885bd22d9ee49d9f80d3017f131f/fxjs/cjs_report.h
[modify] https://crrev.com/ad1f7b410cd6885bd22d9ee49d9f80d3017f131f/fxjs/cjs_util.cpp
[modify] https://crrev.com/ad1f7b410cd6885bd22d9ee49d9f80d3017f131f/fxjs/cjs_event.cpp

Labels: Security_Severity-Medium M-67 Security_Impact-Stable
Project Member

Comment 10 by bugdroid1@chromium.org, Jul 11

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

commit 46629048c53ced6353eb92800761deb10a20ffbc
Author: pdfium-chromium-autoroll <pdfium-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Wed Jul 11 16:40:58 2018

Roll src/third_party/pdfium b1a4db5551ca..e7e454da8e38 (3 commits)

https://pdfium.googlesource.com/pdfium.git/+log/b1a4db5551ca..e7e454da8e38


git log b1a4db5551ca..e7e454da8e38 --date=short --no-merges --format='%ad %ae %s'
2018-07-11 art-snake@yandex-team.ru Do not store cross ref v5 obj within document.
2018-07-11 vmiklos@collabora.co.uk Add FPDFFormObj_CountObjects() API
2018-07-11 thestig@chromium.org Check GetObjDefnID() in various JS functions.


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

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

Change-Id: I5cdecfe75e8069f179eca07f9eb873cfa875c756
Reviewed-on: https://chromium-review.googlesource.com/1133338
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@{#574190}
[modify] https://crrev.com/46629048c53ced6353eb92800761deb10a20ffbc/DEPS

Labels: M-68
Status: Fixed (was: Started)
Attached cast_global.pdf file can reproduce same cast bug in JSSpecialPropPut method of cjs_global.cpp. JSSpecialPropDel, JSSpecialPropGet and JSSpecialPropQuery methods also has similar code.

Document Javascript Section of cast_global.pdf
----------------------------------------------
g = new global.constructor;
Object.setPrototypeOf(g,app);
g.cc=true;
cast_global.pdf
2.0 KB Download
Labels: Pri-1
Status: Started (was: Fixed)
I did look through the other CFXJS_Engine::GetObjectPrivate() calls but clearly I did a poor job.
Project Member

Comment 15 by bugdroid1@chromium.org, Jul 12

The following revision refers to this bug:
  https://pdfium.googlesource.com/pdfium/+/6aa2190f70a80b70af7bcfe198041756ed8c803e

commit 6aa2190f70a80b70af7bcfe198041756ed8c803e
Author: Lei Zhang <thestig@chromium.org>
Date: Thu Jul 12 13:15:11 2018

Use JSGetObject() in even more places.

BUG= chromium:862059 

Change-Id: Id354a5e6dbc037dbb76f901de8311a4f4a4d8940
Reviewed-on: https://pdfium-review.googlesource.com/37670
Commit-Queue: dsinclair <dsinclair@chromium.org>
Reviewed-by: dsinclair <dsinclair@chromium.org>

[modify] https://crrev.com/6aa2190f70a80b70af7bcfe198041756ed8c803e/fxjs/cjs_global.cpp

Project Member

Comment 16 by bugdroid1@chromium.org, Jul 12

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

commit df1df391ec8f9534d21219ea753c260c4357588b
Author: pdfium-chromium-autoroll <pdfium-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Thu Jul 12 14:29:37 2018

Roll src/third_party/pdfium 5ff09fb5ee90..6aa2190f70a8 (1 commits)

https://pdfium.googlesource.com/pdfium.git/+log/5ff09fb5ee90..6aa2190f70a8


git log 5ff09fb5ee90..6aa2190f70a8 --date=short --no-merges --format='%ad %ae %s'
2018-07-12 thestig@chromium.org Use JSGetObject() in even more places.


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

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

Change-Id: I1f8681742658f04204fb7df938c0667e814d84c6
Reviewed-on: https://chromium-review.googlesource.com/1134951
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@{#574556}
[modify] https://crrev.com/df1df391ec8f9534d21219ea753c260c4357588b/DEPS

Status: Fixed (was: Started)
Hope I got all the bad casts. I also have 2 more cleanup CLs to consolidate CFXJS_Engine::GetObjectPrivate() calls based on the most common usage patterns. So the code base will be down to only 3 callers soon.

https://pdfium-review.googlesource.com/c/pdfium/+/37513
https://pdfium-review.googlesource.com/c/pdfium/+/37811
Project Member

Comment 19 by sheriffbot@chromium.org, Jul 13

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

Comment 21 by sheriffbot@chromium.org, Jul 13

Labels: -Merge-Request-68 Hotlist-Merge-Review Merge-Review-68
This bug requires manual review: We are only 10 days from stable.
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), kariahda@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Probably not going to merge to M67, given it's 10 days until M68 stable.
Labels: reward-topanel
Labels: -M-67
How safe is this merge overall to M68?
Might also be nice to not even take the
   // It could be a global proxy object.
path unless we know that the ObjDefnID corresponds to an object definition of type FXJSOBJTYPE_GLOBAL.
re: comment 24: Should be safe.
re: comment 25: Care to send a CL for that?
Labels: -Merge-Review-68 Merge-Approved-68
Approved- branch:3440
Project Member

Comment 28 by bugdroid1@chromium.org, Jul 17

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

commit ff402c2c4ce8ae8690959262ca731d5cc6bd7015
Author: Tom Sepez <tsepez@chromium.org>
Date: Tue Jul 17 00:12:56 2018

Check for global flag on global proxy objects.

Second line of defense for issue in the associated bug.

Bug:  chromium:862059 
Change-Id: I58ba890dfe02c89dd6bcfa23e2e116e107f9adbc
Reviewed-on: https://pdfium-review.googlesource.com/37991
Commit-Queue: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>

[modify] https://crrev.com/ff402c2c4ce8ae8690959262ca731d5cc6bd7015/fxjs/cfxjs_engine.cpp

Project Member

Comment 29 by bugdroid1@chromium.org, Jul 17

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

commit 2a02c684f8fa69e776995b77e183ce6a597fa6e3
Author: pdfium-chromium-autoroll <pdfium-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Tue Jul 17 02:37:59 2018

Roll src/third_party/pdfium 481749905d44..ff402c2c4ce8 (11 commits)

https://pdfium.googlesource.com/pdfium.git/+log/481749905d44..ff402c2c4ce8


git log 481749905d44..ff402c2c4ce8 --date=short --no-merges --format='%ad %ae %s'
2018-07-17 tsepez@chromium.org Check for global flag on global proxy objects.
2018-07-16 tsepez@chromium.org Make JSGetObject<C>() return UnownedPtr<C>.
2018-07-16 tsepez@chromium.org Use UnownedPtr/Optional in cfxa_layoutcontext.cpp
2018-07-16 tsepez@chromium.org Use UnownedPtr<> to v8::Isolates.
2018-07-16 hnakashima@chromium.org Fix crash when typing letters into an XFA datetime field.
2018-07-16 rharrison@chromium.org Alert embedder when attempting to save XFA form
2018-07-16 thestig@chromium.org Fix some nits in CPDF_Document.
2018-07-16 vmiklos@collabora.co.uk Add FPDFFormObj_GetObject() API
2018-07-16 rharrison@chromium.org Process data changes regardless if they can be formatted
2018-07-16 tsepez@chromium.org Use UnownedPtr in CXFA_LocaleMgr
2018-07-16 tsepez@chromium.org Remove unused member from CPDF_DataAvail.


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

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

Change-Id: I34c5767262f39734719d87120febf1c2c8193a9a
Reviewed-on: https://chromium-review.googlesource.com/1139006
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@{#575525}
[modify] https://crrev.com/2a02c684f8fa69e776995b77e183ce6a597fa6e3/DEPS

Project Member

Comment 30 by bugdroid1@chromium.org, Jul 17

Labels: -merge-approved-68 merge-merged-3440
The following revision refers to this bug:
  https://pdfium.googlesource.com/pdfium/+/b6b5a2dcd7cc4302d5f20e21ab637be1b03d1019

commit b6b5a2dcd7cc4302d5f20e21ab637be1b03d1019
Author: Lei Zhang <thestig@chromium.org>
Date: Tue Jul 17 21:33:37 2018

M68: Check GetObjDefnID() in various JS functions.

Consolidate all the checks into JSGetObject(), and add GetObjDefnID()
methods for classes that are missing it.

BUG= chromium:862059 

Change-Id: I2c2b725a01dcd259ef712d2513fcf740cc410b15
Reviewed-on: https://pdfium-review.googlesource.com/37510
Commit-Queue: dsinclair <dsinclair@chromium.org>
Reviewed-by: dsinclair <dsinclair@chromium.org>
(cherry picked from commit ad1f7b410cd6885bd22d9ee49d9f80d3017f131f)
Reviewed-on: https://pdfium-review.googlesource.com/38030
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>

[modify] https://crrev.com/b6b5a2dcd7cc4302d5f20e21ab637be1b03d1019/fxjs/cjs_app.cpp
[modify] https://crrev.com/b6b5a2dcd7cc4302d5f20e21ab637be1b03d1019/fxjs/cjs_console.h
[modify] https://crrev.com/b6b5a2dcd7cc4302d5f20e21ab637be1b03d1019/fxjs/JS_Define.h
[modify] https://crrev.com/b6b5a2dcd7cc4302d5f20e21ab637be1b03d1019/fxjs/cjs_color.h
[modify] https://crrev.com/b6b5a2dcd7cc4302d5f20e21ab637be1b03d1019/fxjs/cjs_console.cpp
[modify] https://crrev.com/b6b5a2dcd7cc4302d5f20e21ab637be1b03d1019/fxjs/cjs_event.h
[modify] https://crrev.com/b6b5a2dcd7cc4302d5f20e21ab637be1b03d1019/fxjs/cjs_app.h
[modify] https://crrev.com/b6b5a2dcd7cc4302d5f20e21ab637be1b03d1019/fxjs/cjs_util.h
[modify] https://crrev.com/b6b5a2dcd7cc4302d5f20e21ab637be1b03d1019/fxjs/cjs_report.cpp
[modify] https://crrev.com/b6b5a2dcd7cc4302d5f20e21ab637be1b03d1019/fxjs/cjs_global.cpp
[modify] https://crrev.com/b6b5a2dcd7cc4302d5f20e21ab637be1b03d1019/fxjs/cjs_global.h
[modify] https://crrev.com/b6b5a2dcd7cc4302d5f20e21ab637be1b03d1019/fxjs/cjs_color.cpp
[modify] https://crrev.com/b6b5a2dcd7cc4302d5f20e21ab637be1b03d1019/fxjs/cjs_report.h
[modify] https://crrev.com/b6b5a2dcd7cc4302d5f20e21ab637be1b03d1019/fxjs/cjs_util.cpp
[modify] https://crrev.com/b6b5a2dcd7cc4302d5f20e21ab637be1b03d1019/fxjs/cjs_event.cpp

Project Member

Comment 31 by bugdroid1@chromium.org, Jul 17

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

commit a1c1c63d3454e51ec321cdaa6e3420db2ed6c957
Author: Lei Zhang <thestig@chromium.org>
Date: Tue Jul 17 21:34:27 2018

M68: Use JSGetObject() in even more places.

This is a manual merge of commit 6aa2190.

BUG= chromium:862059 

Change-Id: Iae0069f1532adc8aea3ac865a4242b00b84d632b
Reviewed-on: https://pdfium-review.googlesource.com/38050
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>

[modify] https://crrev.com/a1c1c63d3454e51ec321cdaa6e3420db2ed6c957/fxjs/cjs_global.cpp

Exploitablity of this bug
---------------------------
Sometimes it is possible to read some values from memory. But not consistently. I attached a test case to demonstrate this issue.

OS: Linux (Because test case depends a lot on behavior of tcmalloc)
    Windows (Shows memory values on very rare occasions)
Chrome Version: 67.0.3396.99 stable

* If you are testing on a debug build, please build tcmalloc with gn flag 
  enable_debugallocation=false. Also debug build should not contain the fix.
 
Steps:
1. Open cast_read.pdf file with chrome.
2. Click "Show Memory" button.
   Wait for about 6-7 seconds.
   PDF files tries to read memory values through "color.red", "color.green" etc. Javascript property accesses. These properties return a Javascript array. If these returned arrays contain values from memory, PDF file will display an Alert with those values.
  If it cannot find any memory values PDF file will display an Alert with a message.
3. If this test case cannot display any memory values please click "Show Memory" button again.
   Also try to close the browser and start again from step 1.

Attached description.png shows what this test case tries to do.
But I am not sure whether that is what happens exactly because returned data is very inconsistent.

Javascript for Test Case
------------------------
OnClick Handler of btn1(Show Memory) button contains the Javascript.
I have added some comments there too.



   




cast_read.pdf
4.0 KB Download
description.png
21.5 KB View Download

Comment 33 Deleted

Correction on Comment 32 (Exploitablity of this bug)
----------------------------------------------------
Test case attached on comment 32 displays values in memory for stable and release builds. But not for the exact reason I mentioned in description.png. CJS_Icon and ArrayBuffer objects are not allocated on adjacent memory locations on release builds as shown in description.png.

Reason: CJS_Icon takes 48 bytes. On a debug build tcmalloc uses size class 64 for CJS_Icon. But on a release build tcmalloc uses size class 48.

Please try attached cast_read_release.pdf, if above test case does not work on stable build. This test case works, but not for the exact reason mentioned in description.png.
cast_read_release.pdf
4.1 KB Download
Labels: Release-0-M68
Labels: -Security_Severity-Medium Security_Severity-High
Labels: -reward-topanel reward-unpaid reward-5000
*** 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.
*********************************
Nice one! $5,000 for this report :)
Labels: -reward-unpaid reward-inprocess
Labels: CVE-2018-6170 CVE_description-missing
Project Member

Comment 41 by sheriffbot@chromium.org, Oct 19

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