Issue metadata
Sign in to add a comment
|
Use-after-poison in blink::ExecutionContext::isContextDestroyed |
||||||||||||||||||||||
Issue descriptionDetailed report: https://clusterfuzz.com/testcase?key=6459015279411200 Fuzzer: inferno_layout_test_unmodified Job Type: linux_debug_content_shell_drt Platform Id: linux Crash Type: Use-after-poison READ 1 Crash Address: 0x7eca51981929 Crash State: blink::ExecutionContext::isContextDestroyed blink::ContextClient::getExecutionContext blink::IDBTransaction::getExecutionContext Sanitizer: address (ASAN) Regressed: https://clusterfuzz.com/revisions?job=linux_debug_content_shell_drt&range=455446:455449 Reproducer Testcase: https://clusterfuzz.com/download/AMIfv964Qr3pfLjEaLUG0_lngcv79PdCHFI0IuwYGh0MH9BxFlE0XpHKi3JMp1hiJ05GDtxvodkOHt5gk4F1sEkStBcrrv4Q3TBXOdq9Uts3pPEGlWZN6H-S2iCzQ_WuH9mtNngiJJy-a1CBlmnA1V_VsXaiI6Q8nUhv_bFll3yYuc2cRw_SRf6yKsyYBii0n4s0oJMU90qCfksSOhlzIlBNahsV47CjGaLFtP1S-aK4jM1-2lFNHcxQAW6wagF5P-SWiq4s2Ap2hWayTm2Mv7XeSbYJnU4hCdKeDFuR6jNAyENX3KpaTvX50USYmvFBKR_8hLENCBSkmm2P_OcR4-rlDsk5CkJ7sppoOCPBvszN45zrC0pjH9s?testcase_id=6459015279411200 Issue filed automatically. See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.
,
Mar 9 2017
This issue is a security regression. If you are not able to fix this quickly, please revert the change that introduced it. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Mar 9 2017
,
Mar 10 2017
,
Mar 13 2017
The blame for this bug is surprisingly tiny and there's not a lot of relevant stuff in there. Adding people who've made the most recent change to lifecycle observers and IndexDB stuff.
,
Mar 13 2017
The ContextClient's WeakMember<> will normally be cleared to prevent any such poisoned access. Understanding why that doesn't hold here, is likely the key.
,
Mar 13 2017
#0 0x7f159035d17b in blink::ExecutionContext::isContextDestroyed() const third_party/WebKit/Source/core/dom/ExecutionContext.h:151:44 #1 0x7f159170a526 in blink::ContextClient::getExecutionContext() const third_party/WebKit/Source/core/dom/ContextLifecycleObserver.cpp:20:53 #2 0x7f158b625275 in blink::IDBTransaction::getExecutionContext() const third_party/WebKit/Source/modules/indexeddb/IDBTransaction.cpp:482:25 #3 0x7f158b624e57 in blink::IDBTransaction::~IDBTransaction() third_party/WebKit/Source/modules/indexeddb/IDBTransaction.cpp:167:3 #4 0x7f15894d52ba in blink::GarbageCollectedFinalized<blink::EventTarget>::finalizeGarbageCollectedObject() third_party/WebKit/Source/platform/heap/GarbageCollected.h:216:67 #5 0x7f15894d5221 in blink::FinalizerTraitImpl<blink::EventTarget, true>::finalize(void*) third_party/WebKit/Source/platform/heap/GCInfo.h:36:27 #6 0x7f15894d51e1 in blink::FinalizerTrait<blink::EventTarget>::finalize(void*) third_party/WebKit/Source/platform/heap/GCInfo.h:62:5 #7 0x7f159fc68fc4 in blink::HeapObjectHeader::finalize(unsigned char*, unsigned long) third_party/WebKit/Source/platform/heap/HeapPage.cpp:103:5 #8 0x7f159fc7f78a in blink::NormalPage::sweep() third_party/WebKit/Source/platform/heap/HeapPage.cpp:1330:15 #9 0x7f159fc6dc59 in blink::BaseArena::sweepUnsweptPage() third_party/WebKit/Source/platform/heap/HeapPage.cpp:278:11 This is happening during a destructor of IDBTransaction. Hmm. It should be safe to access a weak member during a destructor...?
,
Mar 13 2017
Yes, either it will be alive and accessible, or nulled out by weak processing before any destructors run. Which we depend on all the time, so wonder what's going wrong here.
,
Mar 13 2017
,
Mar 13 2017
Ah, the ExecutionContext and IDBTransaction could be swept out by the same GC cycle, hence no weak processing.
,
Mar 13 2017
Ah, good point. So we should not touch a weak member during a destructor.
,
Mar 13 2017
Yes, the DCHECK()s in there might be too strong with IDBTransaction now being a ContextClient. I can take a look.
,
Mar 13 2017
This is a regression from https://codereview.chromium.org/2721603005/ , will just go back to ContextLifecycleObserver for IDBTransaction. (DCHECK()-only, so no production build impact.)
,
Mar 13 2017
bugdroid1 may end up adding an entry referring to https://codereview.chromium.org/2742393002 , which has the fix.
,
Mar 13 2017
,
Mar 14 2017
,
Apr 28 2017
,
Apr 28 2017
+ awhalley@ for security review.
,
Apr 28 2017
abdulsyed@ - Good for 59
,
Apr 29 2017
Your change meets the bar and is auto-approved for M59. Please go ahead and merge the CL to branch 3071 manually. Please contact milestone owner if you have questions. Owners: amineer@(Android), cmasso@(iOS), gkihumba@(ChromeOS), Abdul Syed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
May 2 2017
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible! If all merges have been completed, please remove any remaining Merge-Approved labels from this issue. Thanks for your time! To disable nags, add the Disable-Nags label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
May 5 2017
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible! If all merges have been completed, please remove any remaining Merge-Approved labels from this issue. Thanks for your time! To disable nags, add the Disable-Nags label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
May 11 2017
Marking this as merged, appears to have been merged already. https://chromium.googlesource.com/chromium/src/+/5dec7437ab22a52aa3dcab8e3415142b6db89040
,
May 12 2017
Please merge your change to M59 branch 3071 by 4:00 PM PT, Monday (05/15) so we can take it in for next week beta release. Thank you.
,
May 17 2017
,
Jun 20 2017
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 |
|||||||||||||||||||||||
Comment 1 by sheriffbot@chromium.org
, Mar 9 2017