hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-29 02:35:15 +02:00
Code Issues Packages Projects Releases Wiki Activity

JS: whitelist _.bindAll-methods in js/unbound-event-handler-receiver

Browse Source
This commit is contained in:
Esben Sparre Andreasen
2018-09-11 21:37:02 +02:00
parent 9e0ba51280
commit 1220b50737
3 changed files with 49 additions and 22 deletions
Download Patch File Download Diff File Expand all files Collapse all files

20
javascript/ql/src/Expressions/UnboundEventHandlerReceiver.ql
View File

@@ -13,16 +13,30 @@ import javascript
* Holds if the receiver of `method` is bound in a method of its class.
*/
private predicate isBoundInMethod(MethodDeclaration method) {
exists (DataFlow::ThisNode thiz, MethodDeclaration bindingMethod |
exists (DataFlow::ThisNode thiz, MethodDeclaration bindingMethod, string name |
name = method.getName() and
bindingMethod.getDeclaringClass() = method.getDeclaringClass() and
not bindingMethod.isStatic() and
thiz.getBinder().getAstNode() = bindingMethod.getBody() and
thiz.getBinder().getAstNode() = bindingMethod.getBody() |
exists (DataFlow::Node rhs, DataFlow::MethodCallNode bind |
// this.<methodName> = <expr>.bind(...)
thiz.hasPropertyWrite(method.getName(), rhs) and
thiz.hasPropertyWrite(name, rhs) and
bind.flowsTo(rhs) and
bind.getMethodName() = "bind"
)
or
exists (DataFlow::MethodCallNode bindAll |
bindAll.getMethodName() = "bindAll" and
thiz.flowsTo(bindAll.getArgument(0)) |
// _.bindAll(this, <name1>)
bindAll.getArgument(1).mayHaveStringValue(name)
or
// _.bindAll(this, [<name1>, <name2>])
exists (DataFlow::ArrayLiteralNode names |
names.flowsTo(bindAll.getArgument(1)) and
names.getAnElement().mayHaveStringValue(name)
)
)
)
}

6
javascript/ql/test/query-tests/Expressions/UnboundEventHandlerReceiver/UnboundEventHandlerReceiver.expected
View File

@@ -1,3 +1,3 @@
| tst.js:56:18:56:40 | onClick ... bound1} | The receiver of this event handler call is unbound, `$@` will be `undefined` in the call to $@ | tst.js:14:9:14:12 | this | this | tst.js:13:5:15:5 | unbound ... ;\\n } | unbound1 |
| tst.js:57:18:57:40 | onClick ... bound2} | The receiver of this event handler call is unbound, `$@` will be `undefined` in the call to $@ | tst.js:18:15:18:18 | this | this | tst.js:17:5:19:5 | unbound ... ;\\n } | unbound2 |
| tst.js:58:18:58:35 | onClick={unbound3} | The receiver of this event handler call is unbound, `$@` will be `undefined` in the call to $@ | tst.js:22:15:22:18 | this | this | tst.js:21:5:23:5 | unbound ... ;\\n } | unbound3 |
| tst.js:8:18:8:40 | onClick ... bound1} | The receiver of this event handler call is unbound, `$@` will be `undefined` in the call to $@ | tst.js:35:9:35:12 | this | this | tst.js:34:5:36:5 | unbound ... ;\\n } | unbound1 |
| tst.js:9:18:9:40 | onClick ... bound2} | The receiver of this event handler call is unbound, `$@` will be `undefined` in the call to $@ | tst.js:39:15:39:18 | this | this | tst.js:38:5:40:5 | unbound ... ;\\n } | unbound2 |
| tst.js:10:18:10:35 | onClick={unbound3} | The receiver of this event handler call is unbound, `$@` will be `undefined` in the call to $@ | tst.js:43:15:43:18 | this | this | tst.js:42:5:44:5 | unbound ... ;\\n } | unbound3 |

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

@@ -1,6 +1,25 @@
import React from 'react';
class Component extends React.Component {
render() {
var unbound3 = this.unbound3;
return <div>
<div onClick={this.unbound1}/> // NOT OK
<div onClick={this.unbound2}/> // NOT OK
<div onClick={unbound3}/> // NOT OK
<div onClick={this.bound_throughBindInConstructor}/> // OK
<div onClick={this.bound_throughDeclaration}/> // OK
<div onClick={this.unbound_butNoThis}/> // OK
<div onClick={this.unbound_butNoThis2}/> // OK
<div onClick={(e) => this.unbound_butInvokedSafely(e)}/> // OK
<div onClick={this.bound_throughBindInMethod}/> // OK
<div onClick={this.bound_throughNonSyntacticBindInConstructor}/> // OK
<div onClick={this.bound_throughBindAllInConstructor1}/> // OK
<div onClick={this.bound_throughBindAllInConstructor2}/> // OK
</div>
}
constructor(props) {
super(props);
this.bound_throughBindInConstructor = this.bound_throughBindInConstructor.bind(this);
@@ -8,6 +27,8 @@ class Component extends React.Component {
var cmp = this;
var bound = (cmp.bound_throughNonSyntacticBindInConstructor.bind(this));
(cmp).bound_throughNonSyntacticBindInConstructor = bound;
_.bindAll(this, 'bound_throughBindAllInConstructor1');
_.bindAll(this, ['bound_throughBindAllInConstructor2']);
}
unbound1() {
@@ -50,22 +71,6 @@ class Component extends React.Component {
this.setState({ });
}
render() {
var unbound3 = this.unbound3;
return <div>
<div onClick={this.unbound1}/> // NOT OK
<div onClick={this.unbound2}/> // NOT OK
<div onClick={unbound3}/> // NOT OK
<div onClick={this.bound_throughBindInConstructor}/> // OK
<div onClick={this.bound_throughDeclaration}/> // OK
<div onClick={this.unbound_butNoThis}/> // OK
<div onClick={this.unbound_butNoThis2}/> // OK
<div onClick={(e) => this.unbound_butInvokedSafely(e)}/> // OK
<div onClick={this.bound_throughBindInMethod}/> // OK
<div onClick={this.bound_throughNonSyntacticBindInConstructor}/> // OK
</div>
}
componentWillMount() {
this.bound_throughBindInMethod = this.bound_throughBindInMethod.bind(this);
}
@@ -74,6 +79,14 @@ class Component extends React.Component {
this.setState({ });
}
bound_throughBindAllInConstructor1() {
this.setState({ });
}
bound_throughBindAllInConstructor2() {
this.setState({ });
}
}
// semmle-extractor-options: --experimental