Address review comments.

Max Schaefer · Max Schaefer · commit 3fe249f25c69 · 2021-02-25T10:48:23.000Z
diff --git a/javascript/ql/src/semmle/javascript/DOM.qll b/javascript/ql/src/semmle/javascript/DOM.qll
@@ -355,8 +355,16 @@ module DOM {
         )
         or
         // A receiver node of an event handler on a DOM node
-        exists(string handler | handler.matches("on%") |
-          this = domValueRef().getAPropertySource(handler).(DataFlow::FunctionNode).getReceiver()
+        exists(DataFlow::SourceNode domNode, DataFlow::FunctionNode eventHandler |
+          // NOTE: we do not use `getABoundFunctionValue()`, since bound functions tend to have
+          // a different receiver anyway
+          eventHandler = domNode.getAPropertySource(any(string n | n.matches("on%")))
+          or
+          eventHandler =
+            domNode.getAMethodCall("addEventListener").getArgument(1).getAFunctionValue()
+        |
+          domNode = domValueRef() and
+          this = eventHandler.getReceiver()
         )
         or
         this = DataFlow::thisNode(any(EventHandlerCode evt))
diff --git a/javascript/ql/test/library-tests/DOM/Customizations.expected b/javascript/ql/test/library-tests/DOM/Customizations.expected
@@ -1,10 +1,10 @@
 test_documentRef
 | customization.js:2:13:2:31 | customGetDocument() |
 | event-handler-receiver.js:1:1:1:8 | document |
+| event-handler-receiver.js:5:1:5:8 | document |
 | nameditems.js:1:1:1:8 | document |
 test_locationRef
 | customization.js:3:3:3:14 | doc.location |
-| event-handler-receiver.js:2:49:2:56 | location |
 test_domValueRef
 | customization.js:4:3:4:20 | doc.getElementById |
 | customization.js:4:3:4:28 | doc.get ... 'test') |
@@ -13,6 +13,10 @@ test_domValueRef
 | event-handler-receiver.js:1:1:1:32 | documen ... my-id') |
 | event-handler-receiver.js:1:44:1:43 | this |
 | event-handler-receiver.js:2:3:2:17 | this.parentNode |
+| event-handler-receiver.js:5:1:5:23 | documen ... entById |
+| event-handler-receiver.js:5:1:5:32 | documen ... my-id') |
+| event-handler-receiver.js:5:60:5:59 | this |
+| event-handler-receiver.js:6:3:6:17 | this.parentNode |
 | nameditems.js:1:1:1:23 | documen ... entById |
 | nameditems.js:1:1:1:30 | documen ... ('foo') |
 | nameditems.js:1:1:2:19 | documen ... em('x') |
diff --git a/javascript/ql/test/library-tests/DOM/event-handler-receiver.html b/javascript/ql/test/library-tests/DOM/event-handler-receiver.html
@@ -3,4 +3,4 @@
 <body>
   <button onclick="alert(this.tagName);">Click me</button>
 </body>
-</html>
+</html>
diff --git a/javascript/ql/test/library-tests/DOM/event-handler-receiver.js b/javascript/ql/test/library-tests/DOM/event-handler-receiver.js
@@ -1,3 +1,7 @@
-document.getElementById('my-id').onclick = function() {
-  this.parentNode.innerHTML = '<h2><a href="' + location.href + '">A link</a></h2>'; // NOT OK
+document.getElementById('my-id').onclick = function () {
+  this.parentNode.innerHTML = '<b>hello</b>'; // `this` is a DOM element
 };
+
+document.getElementById('my-id').addEventListener("click", function (ev) {
+  this.parentNode.innerHTML = '<b>hello</b>'; // `this` is a DOM element
+});

-Original file line number
+Diff line change
 <body>
   <button onclick="alert(this.tagName);">Click me</button>
 </body>
 -</html>
 +</html>