Merge pull request #767 from esben-semmle/js/unknown-bound-event-handler-receiver

Approved by xiemaisi

Author: semmle-qlci

change-notes/1.20/analysis-javascript.md

@@ -19,6 +19,7 @@
 | Incomplete URL substring sanitization | correctness, security, external/cwe/cwe-020 | Highlights URL sanitizers that are likely to be incomplete, indicating a violation of [CWE-020](https://cwe.mitre.org/data/definitions/20.html). Results shown on LGTM by default. |
 | Incorrect suffix check (`js/incorrect-suffix-check`) | correctness, security, external/cwe/cwe-020 | Highlights error-prone suffix checks based on `indexOf`, indicating a potential violation of [CWE-20](https://cwe.mitre.org/data/definitions/20.html). Results are shown on LGTM by default. |
 | Loop iteration skipped due to shifting (`js/loop-iteration-skipped-due-to-shifting`) | correctness | Highlights code that removes an element from an array while iterating over it, causing the loop to skip over some elements. Results are shown on LGTM by default. |
+| Unbound event handler receiver (`js/unbound-event-handler-receiver`) | Fewer false positive results | Additional ways that class methods can be bound are recognized. |
 | Useless comparison test (`js/useless-comparison-test`) | correctness | Highlights code that is unreachable due to a numeric comparison that is always true or always false. Results are shown on LGTM by default. |
 
 ## Changes to existing queries

javascript/ql/src/Expressions/UnboundEventHandlerReceiver.ql

@@ -18,7 +18,15 @@ private predicate isBoundInMethod(MethodDeclaration method) {
     bindingMethod.getDeclaringClass() = method.getDeclaringClass() and
     not bindingMethod.isStatic() and
     thiz.getBinder().getAstNode() = bindingMethod.getBody()
-  |
+    |
+    exists (DataFlow::MethodCallNode bind, DataFlow::PropWrite w |
+      // this[x] = <expr>.bind(...)
+      w = thiz.getAPropertyWrite() and
+      not exists(w.getPropertyName()) and
+      bind.getMethodName() = "bind" and
+      bind.flowsTo(w.getRhs())
+    )
+    or
     // require("auto-bind")(this)
     thiz.flowsTo(DataFlow::moduleImport("auto-bind").getACall().getArgument(0))
     or

javascript/ql/test/query-tests/Expressions/UnboundEventHandlerReceiver/tst.js

@@ -134,4 +134,25 @@ class Component2 extends React.Component {
     }
 
 }
+
+class Component3 extends React.Component {
+
+    render() {
+        return <div>
+            <div onClick={this.bound_throughIterator}/> // OK
+            </div>
+    }
+
+    constructor(props) {
+        super(props);
+	    Object.getOwnPropertyNames( Component3.prototype )
+		    .filter( prop => typeof this[ prop ] === 'function' )
+		    .forEach( prop => ( this[ prop ] = this[ prop ].bind( this ) ) );
+    }
+
+    bound_throughIterator() {
+        this.setState({ });
+    }
+}
+
 // semmle-extractor-options: --experimental