Fixes Null-dereference READ in blink::HTMLAnchorElement::HandleClick

This is a regression introduced by my fix to HTMLAnchorElement when the <a>, with embedded inline element, is placed inside a <summary> tag. https://chromium.googlesource.com/chromium/src/+/4eb4df63f17ee0f22cd17472fa6ae92311e49900 The regression is caught by fuzz test: https://clusterfuzz.com/download?testcase_id=4577041865113600. The fuzz test create a click event on the document object which gets handled by node.cc::DefaultEventHandler(). It creates an DOMActivate event and propagate down to HTMLAreaElement. However, the event target is set to NULL by dom/events/event_dispatcher.cc. https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/dom/events/event_dispatcher.cc?q=event_dispatcher.cc&sq=package:chromium&dr&l=391 Comment indicates the reason is that the event is crossing the shadow DOM boundary. Eventually, inside HTMLAnchorElement::HandleClick -> HTMLAnchorElement::AppendServerMapMousePosition() expects target to be non NULL and Null dereference exception is thrown. The fix is to check the event target for null before handling the underlying event. I went through the call stack to ensure no other event members are needed. I verified that both newly created anchor test and fuzz test pass. Bug: 1029197 Change-Id: I19a3b2811c1d562f4c563bed54ae380d7849b06d Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1945852 Commit-Queue: Yu Han <yuzhehan@chromium.org> Reviewed-by: Kent Tamura <tkent@chromium.org> Reviewed-by: Mason Freed <masonfreed@chromium.org> Cr-Commit-Position: refs/heads/master@{#721609}

Fixes Null-dereference READ in blink::HTMLAnchorElement::HandleClick
This is a regression introduced by my fix to HTMLAnchorElement when the <a>, with embedded inline element, is placed inside a <summary> tag. https://chromium.googlesource.com/chromium/src/+/4eb4df63f17ee0f22cd17472fa6ae92311e49900 The regression is caught by fuzz test: https://clusterfuzz.com/download?testcase_id=4577041865113600. The fuzz test create a click event on the document object which gets handled by node.cc::DefaultEventHandler(). It creates an DOMActivate event and propagate down to HTMLAreaElement. However, the event target is set to NULL by dom/events/event_dispatcher.cc. https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/dom/events/event_dispatcher.cc?q=event_dispatcher.cc&sq=package:chromium&dr&l=391 Comment indicates the reason is that the event is crossing the shadow DOM boundary. Eventually, inside HTMLAnchorElement::HandleClick -> HTMLAnchorElement::AppendServerMapMousePosition() expects target to be non NULL and Null dereference exception is thrown. The fix is to check the event target for null before handling the underlying event. I went through the call stack to ensure no other event members are needed. I verified that both newly created anchor test and fuzz test pass. Bug: 1029197 Change-Id: I19a3b2811c1d562f4c563bed54ae380d7849b06d Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1945852 Commit-Queue: Yu Han <yuzhehan@chromium.org> Reviewed-by: Kent Tamura <tkent@chromium.org> Reviewed-by: Mason Freed <masonfreed@chromium.org> Cr-Commit-Position: refs/heads/master@{#721609}
c5443103 · yu han · Commit Bot · e2110405 · c5443103 · c5443103
Commit c5443103 authored Dec 04, 2019 by yu han Committed by Commit Bot Dec 04, 2019
2 changed files
--- a/third_party/blink/renderer/core/html/html_anchor_element.cc
+++ b/third_party/blink/renderer/core/html/html_anchor_element.cc
@@ -459,8 +459,10 @@ bool IsEnterKeyKeydownEvent(Event& event) {
 Event* GetClickEventOrNull(Event& event) {
  // HTMLElement embedded in anchor tag dispatches a DOMActivate event when
  // clicked on. The original click event is set as underlying event.
-  bool use_underlying_event =
-      event.type() == event_type_names::kDOMActivate && event.UnderlyingEvent();
+  bool use_underlying_event = event.type() == event_type_names::kDOMActivate &&
+                              event.UnderlyingEvent() &&
+                              !event.UnderlyingEvent()->DefaultHandled() &&
+                              event.UnderlyingEvent()->target();
  Event* process_event =
      use_underlying_event ? event.UnderlyingEvent() : &event;
  if ((process_event->type() != event_type_names::kClick &&

--- a/third_party/blink/web_tests/fast/dom/shadow/shadow-dom-anchor-click-crash.html
+++ b/third_party/blink/web_tests/fast/dom/shadow/shadow-dom-anchor-click-crash.html
+<!DOCTYPE html>
+<html>
+<head>
+<title>Shadow DOM click inside anchor crash </title>
+<script src="../../../resources/testharness.js"></script>
+<script src="../../../resources/testharnessreport.js"></script>
+</head>
+<body>
+  <!-- [Bug 1029197], https://crbug.com/1029197 -->
+  <!-- [Shadow]: Inside a scoped event, Clicking on a inline element embedded in anchor that's -->
+  <!-- placed inside shadow DOM causes crash.  -->
+  <div id="host"></div>
+  <script>
+    async_test(t => {
+      let sr = host.attachShadow({mode: "open"});
+      sr.innerHTML = '<a href="#"><span id="target">text</span></a>';
+
+      document.addEventListener("selectstart", t.step_func_done(() => {
+        // trigger a click event inside another scoped event.
+        sr.getElementById('target')
+          .dispatchEvent(new MouseEvent('click', {
+            bubbles: true,
+            composed: false
+          }));
+      })) ;
+      document.execCommand("SelectAll");
+    }, "No Crash");
+  </script>
+</body>
+</html>