UAF in ExtensionInstalledBubbleView::GetWindowTitle |
||||||
Issue descriptionFiled by sheriff-o-matic@appspot.gserviceaccount.com on behalf of yukishiino@chromium.org browser_tests failing on chromium.memory/Mac ASan 64 Tests (1) Builders failed on: - Mac ASan 64 Tests (1): https://ci.chromium.org/buildbot/chromium.memory/Mac%20ASan%2064%20Tests%20%281%29
,
Jun 12 2018
Here is the stack trace:
[ RUN ] DeclarativeApiTest.NoTracesAfterUninstalling
[4397:18183:0611/232711.676023:WARNING:notification_platform_bridge_mac.mm(510)] AlertNotificationService: XPC connection invalidated.
[4403:775:0611/232712.079926:WARNING:vt_video_decode_accelerator_mac.cc(193)] Failed to create VTDecompressionSession: Error Domain=NSOSStatusErrorDomain Code=-8973 "codecOpenErr" (-8973)
[4403:775:0611/232712.080960:WARNING:vt_video_decode_accelerator_mac.cc(215)] Hardware decoding with VideoToolbox is not supported
[4403:775:0611/232712.192337:ERROR:vt_video_encode_accelerator_mac.cc(516)] VTCompressionSessionCreate failed: -12908
[4397:775:0611/232712.474963:WARNING:gaia_auth_fetcher.cc(902)] Could not reach Google Accounts servers: errno -11
[4403:775:0611/232713.051907:WARNING:ca_layer_tree_coordinator.mm(55)] Blank frame: No overlays or CALayers
[4397:775:0611/232713.338807:WARNING:gaia_auth_fetcher.cc(902)] Could not reach Google Accounts servers: errno -11
[4397:775:0611/232716.613037:WARNING:gaia_auth_fetcher.cc(902)] Could not reach Google Accounts servers: errno -11
=================================================================
==4397==ERROR: AddressSanitizer: heap-use-after-free on address 0x6160000e1390 at pc 0x0001238fa7e3 bp 0x7ffee2a12b70 sp 0x7ffee2a12b68
READ of size 1 at 0x6160000e1390 thread T0
#0 0x1238fa7e2 in non-virtual thunk to ExtensionInstalledBubbleView::GetWindowTitle() const ??:0:0
#1 0x11c136d39 in views::WidgetDelegate::GetAccessibleWindowTitle() const ??:0:0
#2 0x11c11d5be in views::internal::RootView::GetAccessibleNodeData(ui::AXNodeData*) ??:0:0
#3 0x11bf0895a in views::ViewAccessibility::GetAccessibleNodeData(ui::AXNodeData*) const ??:0:0
#4 0x118fa92ad in (anonymous namespace)::DoesViewHaveAccessibilityErrorsRecursive(views::View*, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >*) ??:0:0
#5 0x118fa8e7d in AddFailureOnWidgetAccessibilityError(views::Widget*) ??:0:0
#6 0x11c12c869 in views::Widget::OnNativeWidgetVisibilityChanged(bool) ??:0:0
#7 0x11bf7031f in views::BridgedNativeWidget::OnVisibilityChanged() ??:0:0
#8 0x11bf80800 in -[NativeWidgetMacNSWindow orderWindow:relativeTo:] ??:0:0
#9 0x11bf70ca3 in views::BridgedNativeWidget::NotifyVisibilityChangeDown() ??:0:0
#10 0x11bf7027d in views::BridgedNativeWidget::OnVisibilityChanged() ??:0:0
#11 0x11bf80800 in -[NativeWidgetMacNSWindow orderWindow:relativeTo:] ??:0:0
#12 0x11bf6c059 in views::BridgedNativeWidget::SetVisibilityState(views::BridgedNativeWidget::WindowVisibilityState) ??:0:0
#13 0x123a8f8e9 in BrowserView::CanClose() ??:0:0
#14 0x11c12806c in views::Widget::Close() ??:0:0
#15 0x119c8cadb in BrowserCloseManager::CloseBrowsers() ??:0:0
#16 0x119c8d2e3 in BrowserCloseManager::TryToCloseBrowsers() ??:0:0
#17 0x1196daae9 in chrome::CloseAllBrowsers() ??:0:0
#18 0x11938b6f3 in -[AppController tryToTerminateApplication:] ??:0:0
#19 0x1196db99c in chrome::AttemptExit() ??:0:0
#20 0x118fc3ae8 in base::debug::TaskAnnotator::RunTask(char const*, base::PendingTask*) ??:0:0
#21 0x11904353f in base::internal::IncomingTaskQueue::RunTask(base::PendingTask*) ??:0:0
#22 0x11904e6ee in base::MessageLoop::RunTask(base::PendingTask*) ??:0:0
#23 0x11904f1e0 in base::MessageLoop::DeferOrRunPendingTask(base::PendingTask) ??:0:0
#24 0x11904f8a3 in base::MessageLoop::DoWork() ??:0:0
#25 0x11905972c in base::MessagePumpCFRunLoopBase::RunWork() ??:0:0
#26 0x11900fb99 in base::mac::CallWithEHFrame(void () block_pointer) ??:0:0
#27 0x119057cf5 in base::MessagePumpCFRunLoopBase::RunWorkSource(void*) ??:0:0
This does not reproduce on Linux, and started here on MacOS: https://ci.chromium.org/buildbot/chromium.memory/Mac%20ASan%2064%20Tests%20%281%29/41167
I expect https://chromium.googlesource.com/chromium/src/+/f824b3a3926819d6398b7a7b3da1e251d33417da to be the beginning of this issue. As this happens only on Mac, I am suspecting a difference between mac views and other views the root cause and assigning to Elly as the expert. I heard that there were some issues with Cocoa UI in the past because of the way how MacOS garbage collected UI elements lazily.
CCing also Devlin as OWNER of chrome/browser/ui/extensions/extension_installed_bubble.cc.
I see multiple different options here:
1) Maybe we could cache the extension name for the title in the ExtensionInstalledBubbleView so that it does not access the extension after creation anymore. This would come at the expense of an extra string in memory and could hide some true bug.
2) ExtensionInstalledBubbleObserver::OnExtensionUnloaded should learn about the uninstallation of the extension. It deletes itself and should also destroy the bubble. Therefore, I think that this code is correct.
3) I don't know whether we need to keep the test code as I am not familiar enough with Devlin's plans on the declarative API.
With this, I think that I have to defer to Elly and Devlin.
,
Jun 12 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9464483baa03aedfb1dd1e125a60e2d234931191 commit 9464483baa03aedfb1dd1e125a60e2d234931191 Author: Roger McFarlane <rogerm@chromium.org> Date: Tue Jun 12 17:52:45 2018 [sheriff] Disable 2 DeclarativeApiTest cases for Mac+Asan flakiness TBR: battre@chromium.org Bug: 851854 Change-Id: I6b7e57ef8c739dcbb9fc0135fad8edb5d78b4f8f Reviewed-on: https://chromium-review.googlesource.com/1097504 Reviewed-by: Dominic Battré <battre@chromium.org> Reviewed-by: Roger McFarlane <rogerm@chromium.org> Commit-Queue: Roger McFarlane <rogerm@chromium.org> Cr-Commit-Position: refs/heads/master@{#566497} [modify] https://crrev.com/9464483baa03aedfb1dd1e125a60e2d234931191/chrome/browser/extensions/api/declarative/declarative_apitest.cc
,
Jun 12 2018
,
Jul 23
,
Jul 23
,
Jul 23
I see that a fix was attempted in https://chromium-review.googlesource.com/c/chromium/src/+/1093023, but did not land in want for a better fix. From what I can see, just checking for |bubble_reference_| before accessing the |controller_| might be sufficient. Devlin are you planning to change the code here? Also, cc'ing hcarmona@ who introduced the bubble component.
,
Jul 24
,
Nov 20
*** UI Mass Triage *** |
||||||
►
Sign in to add a comment |
||||||
Comment 1 by yukishiino@chromium.org
, Jun 12 2018Owner: battre@chromium.org
Status: Assigned (was: Available)