Automatically assigning owner based on suspected regression changelist https://chromium.googlesource.com/chromium/src/+/bcfb895bd39c421db37531d48accf340b4c0440f ([ozone/x11] Map X's LeaveNotify to ET_MOUSE_EXITED).

If this is incorrect, please remove the owner and apply the Test-Predator-Wrong-CLs label.

Automatically applying components based on crash stacktrace and information from OWNERS files.

If this is incorrect, please apply the Test-Predator-Wrong-Components label.

This is a critical security issue. If you are not able to fix this quickly, please revert the change that introduced it.

If this doesn't affect a release branch, or has not been properly classified for severity, please update the Security_Impact or Security_Severity labels, and remove the ReleaseBlock label. To disable this altogether, apply ReleaseBlock-NA.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

It there any back trace or steps to reproduce the issue?
What build configuration is needed (GN args, etc)?

Increasing visibility since this has been marked as a critical issue.

The backtrace and test cases are in the detailed report. You may not be able to access it though. I've copied the backtrace below:

#0 0x7fd6aa1e06b3 in AddRefImpl base/memory/ref_counted.h:126:29
#1 0x7fd6aa1e06b3 in AddRef base/memory/ref_counted.h:65
#2 0x7fd6aa1e06b3 in AddRef base/memory/ref_counted.h:312
 #3 0x7fd6aa1e06b3 in ui::X11CursorFactoryOzone::RefImageCursor(void*) ui/ozone/platform/x11/x11_cursor_factory_ozone.cc:71
 #4 0x7fd6aa35a625 in content::WebCursor::GetNativeCursor() content/common/cursors/webcursor_aura.cc:106:14
 #5 0x7fd6ac1907a3 in content::RenderWidgetHostViewAura::UpdateCursorIfOverSelf() content/browser/renderer_host/render_widget_host_view_aura.cc:2061:46
#6 0x7fd6ac19043d in content::RenderWidgetHostViewAura::DisplayCursor(content::WebCursor const&) content/browser/renderer_host/render_widget_host_view_aura.cc:828:3
    #7 0x7fd6abf600b3 in content::CursorManager::UpdateCursor(content::RenderWidgetHostViewBase*, content::WebCursor const&) content/browser/renderer_host/cursor_manager.cc:20:17
    #8 0x7fd6ac14dd99 in DispatchToMethodImpl<content::RenderWidgetHostImpl *, void (content::RenderWidgetHostImpl::*)(const content::WebCursor &), std::__1::tuple<content::WebCursor>, 0> base/tuple.h:52:3
    #9 0x7fd6ac14dd99 in DispatchToMethod<content::RenderWidgetHostImpl *, void (content::RenderWidgetHostImpl::*)(const content::WebCursor &), std::__1::tuple<content::WebCursor> > base/tuple.h:60
    #10 0x7fd6ac14dd99 in DispatchToMethod<content::RenderWidgetHostImpl, void (content::RenderWidgetHostImpl::*)(const content::WebCursor &), void, std::__1::tuple<content::WebCursor> > ipc/ipc_message_templates.h:51
    #11 0x7fd6ac14dd99 in bool IPC::MessageT<ViewHostMsg_SetCursor_Meta, std::__1::tuple<content::WebCursor>, void>::Dispatch<content::RenderWidgetHostImpl, content::RenderWidgetHostImpl, void, void (content::RenderWidgetHostImpl::*)(content::WebCursor const&)>(IPC::Message const*, content::RenderWidgetHostImpl*, content::RenderWidgetHostImpl*, void*, void (content::RenderWidgetHostImpl::*)(content::WebCursor const&)) ipc/ipc_message_templates.h:146
    #12 0x7fd6ac147eef in content::RenderWidgetHostImpl::OnMessageReceived(IPC::Message const&) content/browser/renderer_host/render_widget_host_impl.cc:614:5

sadrul: are you able to help debug this?

I'm attaching the testcase in case that helps tonikitoo.

Any chance this would affect Chrome OS as well as Linux?

Deleted: clusterfuzz-testcase-4589845975662592.zip 60.2 KB

clusterfuzz-testcase-4589845975662592.zip 60.2 KB Download

@palmer, thank you.

Would it be possible to share the GN args used? I have a chromeos off-device build locally, with is_asan enabled, but just loading the test case with it does not crash.

(cc/ @palmer)

Elliot knows quite a bit about X11CursorFactoryOzone, passing his way and moving tonikitoo@igalia.com to cc.

X11CursorFactoryOzone is not used on linux desktop, or on chromeos. So this exact issue should not affect either platform. However, depending on the underlying issue causing this failure: we may have something similar on chromeos (where ozone is used, but not the x11 implementation).

#12: So is X11CursorFactoryOzone dead code, then? Maybe we can just remove it? (Separate from nailing down the root cause.)

If this code is not really reachable from the web or network, or if the root cause is in dead code, this isn't really a Critical.

I can't reproduce this with the clusterfuzz reproduce command, including with:

- -i 10, for 10 retries.
- --disable-xvfb, just in case the virtual x server was preventing this from reproing locally.

I can't reproduce this when making a chromeos asan build and just running the test case.

I've gotten this to crash only once. This appears to be a shutdown crash, not a crash on use. I suspect this is really timing dependent.

Attached is symbolized asan output on the run that I was able to get a UAF.

Deleted: symbolized.txt 26.0 KB

symbolized.txt 26.0 KB View Download

I've gotten it to fail twice during page load. I've gotten it to fail one more time during shutdown. However, I've also had 10 runs at a time where nothing happens.

Very speculatively: piecing the stack trace of the memory originally allocated, then deleted, then accessed, I believe that there's some sort of mishandling of refcounts on X11CursorFactoryOzone::invisible_cursor_. I suspect this mishandling is related to how it gets returned in X11CursorFactoryOzone::CreateImageCursor() to work around a different crash in X11.

OK, but then why now? Minus a simple rename patch from early 2017, all the code in x11_cursor_factory_ozone.cc dates back to 2016. If this was broken for awhile, why is this showing up now? Next up is checking the code path which ferries this refcounted type, ***** as an untyped void* *****, to where it is used.

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

commit f11a5f9c757d6821ee4606d0f380e2de8cb85811
Author: Elliot Glaysher <erg@chromium.org>
Date: Wed Dec 06 21:32:11 2017

Fix UAF in X11CursorFactoryOzone.

Add a reference to the returned invisible cursor when we try to create a
cursor from an invalid bitmap.

X11CursorFactoryOzone is only used in the chromeos-on-linux development
environment. I've double checked that this doesn't occur in
CursorDataFactoryOzone and BitmapCursorFactoryOzone.

Bug:  791243 
Change-Id: Ia12edf1638a420b61eb7f72f3696dabf506a44e0
Reviewed-on: https://chromium-review.googlesource.com/809492
Commit-Queue: Elliot Glaysher <erg@chromium.org>
Reviewed-by: kylechar <kylechar@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522200}
[modify] https://crrev.com/f11a5f9c757d6821ee4606d0f380e2de8cb85811/ui/ozone/platform/x11/x11_cursor_factory_ozone.cc

The above patch (hopefully!) fixes the UAF. It at least fixes a UAF in the same area.

Next up is https://chromium-review.googlesource.com/c/chromium/src/+/811986, which adds a test since this is the sort of thing that could regress easily. (This was pulled out of the patch that landed due to concerns over whether the current ozone test binary could access x11.)

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

commit a3ad4ce982d49a655328cc904edb3467dffe852f
Author: Elliot Glaysher <erg@chromium.org>
Date: Fri Dec 08 20:37:29 2017

Adds a test to cover refcounting in X11CursorFactoryOzone.

Bug:  791243 
Change-Id: I2c97821edd8d84b1ef5ed17d76feb78e3d677355
Reviewed-on: https://chromium-review.googlesource.com/811986
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: kylechar <kylechar@chromium.org>
Commit-Queue: Elliot Glaysher <erg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522863}
[modify] https://crrev.com/a3ad4ce982d49a655328cc904edb3467dffe852f/BUILD.gn
[modify] https://crrev.com/a3ad4ce982d49a655328cc904edb3467dffe852f/testing/buildbot/chromium.chromiumos.json
[modify] https://crrev.com/a3ad4ce982d49a655328cc904edb3467dffe852f/testing/buildbot/gn_isolate_map.pyl
[modify] https://crrev.com/a3ad4ce982d49a655328cc904edb3467dffe852f/testing/buildbot/test_suites.pyl
[modify] https://crrev.com/a3ad4ce982d49a655328cc904edb3467dffe852f/ui/ozone/BUILD.gn
[modify] https://crrev.com/a3ad4ce982d49a655328cc904edb3467dffe852f/ui/ozone/platform/x11/BUILD.gn
[add] https://crrev.com/a3ad4ce982d49a655328cc904edb3467dffe852f/ui/ozone/platform/x11/x11_cursor_factory_ozone_unittest.cc

#18 is simple enough to merge to any branch, but I don't believe there's anything that needs to be merged because this code is only used in development environments.

#22 suggests that this bug is not really Critical, then? Users in the field would not be vulnerable to this?

It isn't. I'm not sure why it was tagged that way in the first place. Users in the field would not be vulnerable to this.

OK, thanks.

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