Author: sanjoy.pal@samsung.com
Component: chromium
Changelist: https://chromium.googlesource.com/chromium/src//+/d758323e4c350c5a17b83171dac357dd91edea33
Time: Wed Dec 10 14:31:07 2014
The CL last changed line 37 of file HTMLMenuItemElement.cpp, which is stack frame 2.

sanjoy.pal@: 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

sanjoy.pal@: Uh oh! This issue is still open and hasn't been updated in the last 45 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

sanjoy.pal: Uh oh! This issue still open and hasn't been updated in the last 52 days. This is a serious vulnerability, and we want to ensure that there's progress. Could you please leave an update with the current status and any potential blockers?

If you're not the right owner for this issue, could you please remove yourself as soon as possible or help us find the right one?

If the issue is fixed or you can't reproduce it, please close the bug. If you've started working on a fix, please set the status to Started.

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

sanjoy.pal: Uh oh! This issue still open and hasn't been updated in the last 67 days. This is a serious vulnerability, and we want to ensure that there's progress. Could you please leave an update with the current status and any potential blockers?

If you're not the right owner for this issue, could you please remove yourself as soon as possible or help us find the right one?

If the issue is fixed or you can't reproduce it, please close the bug. If you've started working on a fix, please set the status to Started.

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

Hey tkent. I am going to CC Rob Buis from Samsung, who is one of their main
contributors - I am not part of Samsung team any longer.

It seems like sanjoy is doing some menuitem work (
https://codereview.chromium.org/1955863002/) and might be able to get to
this p0 issue.

@inferno, I'm not able to download the minimized test case. Can you give permission to download?

Repro from cluster-fuzz.appspot.com:

----------------------------------
script src="../../../../../../resources/testharness.js"</script></script><script>
function fuzz() {
      "drag", function() { try {
} catch (e) {
} }
  document.getElementById("dom-fuzz-6790002").addEventListener(
      "fullscreenerror", function() { try {
  var elt = document.getElementById("dom-fuzz-6790002");
  elt.setAttribute("radiogroup", "G2");
} catch (e) {
} });
  e = document.createEvent("Event");
  var element = document.getElementById("dom-fuzz-6790002");
  e.initEvent("fullscreenerror");
  element.dispatchEvent(e);
  e = document.createEvent("Event");
  e.initEvent("click");
  element.dispatchEvent(e);
}
 setTimeout(fuzz); </script><menuitem id="dom-fuzz-6790002" checked="" type="radio"><menuitem checked=""
      </script>
----------------------------------

<body>
<h1>Menuitem</h1>
<menuitem id="domfuzz6790002" checked="" type="radio"><menuitem checked="">
<script>
function fuzz() {
  document.getElementById("domfuzz6790002").addEventListener(
      "fullscreenerror", function() { 
  var elt = document.getElementById("domfuzz6790002");
  elt.setAttribute("radiogroup", "G2");
 });
  e = document.createEvent("Event");
  var element = document.getElementById("domfuzz6790002");
  e.initEvent("fullscreenerror");
  element.dispatchEvent(e);
  e = document.createEvent("Event");
  e.initEvent("click");
  element.dispatchEvent(e);
}
 fuzz();
</script>
</body>

@tkent, Thanks for the testcase, But, i am not able to reproduce the crash with the above html.

Did you try on Linux ASan?  I think container-overflow is detectable only on Linux ASan.

Yes, I could reproduce this on Linux ASan.

I took a look at the CL of mine, but I couldn't find out any problem with the code. https://chromium.googlesource.com/chromium/src//+/d758323e4c350c5a17b83171dac357dd91edea33 was landed in 2014 and the issue is reported in Feb 29, 2016. Does that mean some other CL else has resulted this crash or is the test case added recently? Basically, I couldn't find out the root cause. As this is a P1 issue, can someone with better knowledge of this area please take a look at this?

I guess the root cause is the AtomicString const reference |group|.  This reference will be broken after removeAttribute(checkedAttr) is called for the same element.

|const AtomicString& group| => |AtomicString group| should fix the problem. 

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

commit 4189a821a05342bf177d00744175e367dba67868
Author: sanjoy.pal <sanjoy.pal@samsung.com>
Date: Tue May 10 12:18:29 2016

Fix ASan container overflow in HTMLMenuItemElement::defaultEventHandler

The root cause is the AtomicString const reference |group|.  This reference will be broken after removeAttribute(checkedAttr) is called for the same element.

BUG= 590619 

Review-Url: https://codereview.chromium.org/1962073003
Cr-Commit-Position: refs/heads/master@{#392584}

[add] https://crrev.com/4189a821a05342bf177d00744175e367dba67868/third_party/WebKit/LayoutTests/fast/dom/HTMLMenuItemElement/menuitem-crash-asan-expected.txt
[add] https://crrev.com/4189a821a05342bf177d00744175e367dba67868/third_party/WebKit/LayoutTests/fast/dom/HTMLMenuItemElement/menuitem-crash-asan.html
[modify] https://crrev.com/4189a821a05342bf177d00744175e367dba67868/third_party/WebKit/Source/core/html/HTMLMenuItemElement.cpp

ClusterFuzz has detected this issue as fixed in range 385470:385989.

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=5359590801145856

Fuzzer: ochang_domfuzzer
Job Type: linux_asan_content_shell_drt
Platform Id: linux

Crash Type: Container-overflow READ 8
Crash Address: 0x6060000f4318
Crash State:
  blink::HTMLMenuItemElement::defaultEventHandler
  blink::EventDispatcher::dispatchEventPostProcess
  blink::EventDispatcher::dispatch
  
Regressed: https://cluster-fuzz.appspot.com/revisions?job=linux_asan_content_shell_drt&range=316591:316716
Fixed: https://cluster-fuzz.appspot.com/revisions?job=linux_asan_content_shell_drt&range=385470:385989

Minimized Testcase (0.73 Kb): https://cluster-fuzz.appspot.com/download/AMIfv9788vy5OxmtmlmRT4VjwGH3m-V9EpPIkLhr367ULcPzF8g7YF68qiKVfRRTCQmMB90H0piWTTkoC_2P4Ot-nDtLeQcbbIg2CVxa3U8QufDxY-8TvrJhZbnRL_515KNajdj53PMX4N1JbiQr8IMDU_C-_QMJvw

See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.

Adding Merge-Triage label for tracking purposes.

Once your fix had sufficient bake time (on canary, dev as appropriate), please nominate your fix for merge by adding the Merge-Request-XX label, where XX is the Chrome milestone.

When your merge is approved by the release manager, please start merging with higher milestone label first. Make sure to re-request merge for every milestone in the label list. You can get branch information on omahaproxy.appspot.com.

- Your friendly ClusterFuzz

Internally discovered medium --> can roll with M52.

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

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

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