From 5b7675d972e13e2e6a7546dbef5c1c7fa278c1a0 Mon Sep 17 00:00:00 2001 From: Asger F Date: Fri, 11 Jan 2019 16:46:45 +0000 Subject: [PATCH 1/7] JS: introduce DataFlow::ClassNode --- .../src/semmle/javascript/dataflow/Nodes.qll | 135 ++++++++++++++++++ .../Classes/ClassNodeConstructor.expected | 7 + .../Classes/ClassNodeConstructor.ql | 4 + .../Classes/ClassNodeInstanceMethod.expected | 5 + .../Classes/ClassNodeInstanceMethod.ql | 4 + .../Classes/ClassNodeStaticMethod.expected | 3 + .../Classes/ClassNodeStaticMethod.ql | 4 + 7 files changed, 162 insertions(+) create mode 100644 javascript/ql/test/library-tests/Classes/ClassNodeConstructor.expected create mode 100644 javascript/ql/test/library-tests/Classes/ClassNodeConstructor.ql create mode 100644 javascript/ql/test/library-tests/Classes/ClassNodeInstanceMethod.expected create mode 100644 javascript/ql/test/library-tests/Classes/ClassNodeInstanceMethod.ql create mode 100644 javascript/ql/test/library-tests/Classes/ClassNodeStaticMethod.expected create mode 100644 javascript/ql/test/library-tests/Classes/ClassNodeStaticMethod.ql diff --git a/javascript/ql/src/semmle/javascript/dataflow/Nodes.qll b/javascript/ql/src/semmle/javascript/dataflow/Nodes.qll index 7fca289ac48c..1af39ecfe663 100644 --- a/javascript/ql/src/semmle/javascript/dataflow/Nodes.qll +++ b/javascript/ql/src/semmle/javascript/dataflow/Nodes.qll @@ -465,3 +465,138 @@ DataFlow::SourceNode moduleMember(string path, string m) { result = DataFlow::ssaDefinitionNode(ssa) ) } + +/** + * A data flow node corresponding to a class definition or a function definition + * acting as a class. + * + * The following patterns are recognized as classes and methods: + * ``` + * class C { + * method() + * } + * + * function F() {} + * + * F.prototype.method = function() {} + * + * F.prototype = { + * method: function() {} + * } + * + * extend(F.prototype, { + * method: function() {} + * }); + * ``` + */ +class ClassNode extends DataFlow::SourceNode, DataFlow::ValueNode { + ClassNode() { + astNode instanceof ClassDefinition + or + astNode instanceof Function and + exists(getAPropertyReference("prototype")) + } + + /** + * Gets the name of the class, if it has one. + */ + string getName() { + result = astNode.(ClassDefinition).getName() + or + result = astNode.(Function).getName() + } + + /** + * Gets a description of the class. + */ + string describe() { + result = astNode.(ClassDefinition).describe() + or + result = astNode.(Function).describe() + } + + /** + * Gets the constructor function of this class. + */ + FunctionNode getConstructor() { + result = astNode.(ClassDefinition).getConstructor().getBody().flow() + or + result = this + } + + /** + * Gets an instance method with the given name, if any. + */ + FunctionNode getInstanceMethod(string name) { + exists(MethodDeclaration method | + method = astNode.(ClassDefinition).getMethod(name) and + not method.isStatic() and + not method.isAmbient() and + not method instanceof ConstructorDeclaration and + result = method.getBody().flow() + ) + or + result = getAPrototypeReference().getAPropertyWrite(name).getRhs().getALocalSource() + } + + /** + * Gets an instance method of this class. + * + * The constructor is not considered an instance method. + */ + FunctionNode getAnInstanceMethod() { + exists(MethodDeclaration method | + method = astNode.(ClassDefinition).getAMethod() and + not method.isStatic() and + not method.isAmbient() and + not method instanceof ConstructorDeclaration and + result = method.getBody().flow() + ) + or + result = getAPrototypeReference().getAPropertyWrite().getRhs().getALocalSource() + } + + /** + * Gets the static method of this class with the given name. + */ + FunctionNode getStaticMethod(string name) { + exists(MethodDeclaration method | + method = astNode.(ClassDefinition).getMethod(name) and + method.isStatic() and + not method.isAmbient() and + result = method.getBody().flow() + ) + or + result = getAPropertyWrite(name).getRhs().getALocalSource() + } + + /** + * Gets a static method of this class. + * + * The constructor is not considered a static method. + */ + FunctionNode getAStaticMethod() { + exists(MethodDeclaration method | + method = astNode.(ClassDefinition).getAMethod() and + method.isStatic() and + not method.isAmbient() and + result = method.getBody().flow() + ) + or + result = getAPropertyWrite().getRhs().getALocalSource() + } + + /** + * Gets a reference to the prototype of this class. + */ + private DataFlow::SourceNode getAPrototypeReference() { + result = getAPropertyRead("prototype") + or + result = getAPropertySource("prototype") + or + exists(ExtendCall call | + call.getDestinationOperand() = getAPropertyRead("prototype") and + result = call.getASourceOperand() + ) + } +} diff --git a/javascript/ql/test/library-tests/Classes/ClassNodeConstructor.expected b/javascript/ql/test/library-tests/Classes/ClassNodeConstructor.expected new file mode 100644 index 000000000000..d850dfce0f1c --- /dev/null +++ b/javascript/ql/test/library-tests/Classes/ClassNodeConstructor.expected @@ -0,0 +1,7 @@ +| fields.js:1:1:4:1 | class C ... = 42\\n} | fields.js:1:9:1:8 | () {} | +| points.js:1:1:18:1 | class P ... ;\\n }\\n} | points.js:2:14:5:3 | (x, y) ... y;\\n } | +| points.js:20:1:33:1 | class C ... ;\\n }\\n} | points.js:21:14:24:3 | (x, y, ... c;\\n } | +| staticConstructor.js:1:1:3:1 | class M ... r"; }\\n} | staticConstructor.js:1:15:1:14 | () {} | +| tst.js:1:9:4:1 | class { ... */ }\\n} | tst.js:2:16:2:50 | () { /* ... r. */ } | +| tst.js:6:1:8:1 | class B ... t); }\\n} | tst.js:7:14:7:38 | () { su ... get); } | +| tst.js:11:1:14:1 | class C ... () {}\\n} | tst.js:11:9:11:8 | () {} | diff --git a/javascript/ql/test/library-tests/Classes/ClassNodeConstructor.ql b/javascript/ql/test/library-tests/Classes/ClassNodeConstructor.ql new file mode 100644 index 000000000000..cba12e5f3daf --- /dev/null +++ b/javascript/ql/test/library-tests/Classes/ClassNodeConstructor.ql @@ -0,0 +1,4 @@ +import javascript + +from DataFlow::ClassNode class_ +select class_, class_.getConstructor() diff --git a/javascript/ql/test/library-tests/Classes/ClassNodeInstanceMethod.expected b/javascript/ql/test/library-tests/Classes/ClassNodeInstanceMethod.expected new file mode 100644 index 000000000000..67429ea3b9d7 --- /dev/null +++ b/javascript/ql/test/library-tests/Classes/ClassNodeInstanceMethod.expected @@ -0,0 +1,5 @@ +| points.js:1:1:18:1 | class P ... ;\\n }\\n} | dist | points.js:7:11:9:3 | () {\\n ... y);\\n } | +| points.js:1:1:18:1 | class P ... ;\\n }\\n} | toString | points.js:11:11:13:3 | () {\\n ... )";\\n } | +| points.js:20:1:33:1 | class C ... ;\\n }\\n} | toString | points.js:26:11:28:3 | () {\\n ... ur;\\n } | +| tst.js:1:9:4:1 | class { ... */ }\\n} | constructor | tst.js:3:18:3:56 | () { /* ... r. */ } | +| tst.js:11:1:14:1 | class C ... () {}\\n} | m | tst.js:12:4:12:8 | () {} | diff --git a/javascript/ql/test/library-tests/Classes/ClassNodeInstanceMethod.ql b/javascript/ql/test/library-tests/Classes/ClassNodeInstanceMethod.ql new file mode 100644 index 000000000000..dd64123052cc --- /dev/null +++ b/javascript/ql/test/library-tests/Classes/ClassNodeInstanceMethod.ql @@ -0,0 +1,4 @@ +import javascript + +from DataFlow::ClassNode class_, string name +select class_, name, class_.getInstanceMethod(name) diff --git a/javascript/ql/test/library-tests/Classes/ClassNodeStaticMethod.expected b/javascript/ql/test/library-tests/Classes/ClassNodeStaticMethod.expected new file mode 100644 index 000000000000..b2e46468dce6 --- /dev/null +++ b/javascript/ql/test/library-tests/Classes/ClassNodeStaticMethod.expected @@ -0,0 +1,3 @@ +| points.js:1:1:18:1 | class P ... ;\\n }\\n} | className | points.js:15:19:17:3 | () {\\n ... t";\\n } | +| points.js:20:1:33:1 | class C ... ;\\n }\\n} | className | points.js:30:19:32:3 | () {\\n ... t";\\n } | +| staticConstructor.js:1:1:3:1 | class M ... r"; }\\n} | constructor | staticConstructor.js:2:21:2:59 | () { re ... tor"; } | diff --git a/javascript/ql/test/library-tests/Classes/ClassNodeStaticMethod.ql b/javascript/ql/test/library-tests/Classes/ClassNodeStaticMethod.ql new file mode 100644 index 000000000000..f3b63ba69ea6 --- /dev/null +++ b/javascript/ql/test/library-tests/Classes/ClassNodeStaticMethod.ql @@ -0,0 +1,4 @@ +import javascript + +from DataFlow::ClassNode class_, string name +select class_, name, class_.getStaticMethod(name) From 3cb2341e63233f51cd546a393824c5b4282f40c9 Mon Sep 17 00:00:00 2001 From: Asger F Date: Mon, 14 Jan 2019 15:04:06 +0000 Subject: [PATCH 2/7] JS: split ClassNode into two classes --- .../src/semmle/javascript/dataflow/Nodes.qll | 199 +++++++++++------- 1 file changed, 123 insertions(+), 76 deletions(-) diff --git a/javascript/ql/src/semmle/javascript/dataflow/Nodes.qll b/javascript/ql/src/semmle/javascript/dataflow/Nodes.qll index 1af39ecfe663..e5d8ba939476 100644 --- a/javascript/ql/src/semmle/javascript/dataflow/Nodes.qll +++ b/javascript/ql/src/semmle/javascript/dataflow/Nodes.qll @@ -489,114 +489,161 @@ DataFlow::SourceNode moduleMember(string path, string m) { * }); * ``` */ -class ClassNode extends DataFlow::SourceNode, DataFlow::ValueNode { - ClassNode() { - astNode instanceof ClassDefinition - or - astNode instanceof Function and - exists(getAPropertyReference("prototype")) - } +class ClassNode extends DataFlow::SourceNode { + ClassNode::Range impl; + + ClassNode() { this = impl } /** * Gets the name of the class, if it has one. */ - string getName() { - result = astNode.(ClassDefinition).getName() - or - result = astNode.(Function).getName() - } + string getName() { result = impl.getName() } /** * Gets a description of the class. */ - string describe() { - result = astNode.(ClassDefinition).describe() - or - result = astNode.(Function).describe() - } + string desribe() { result = impl.describe() } /** * Gets the constructor function of this class. */ - FunctionNode getConstructor() { - result = astNode.(ClassDefinition).getConstructor().getBody().flow() - or - result = this - } + FunctionNode getConstructor() { result = impl.getConstructor() } /** * Gets an instance method with the given name, if any. */ - FunctionNode getInstanceMethod(string name) { - exists(MethodDeclaration method | - method = astNode.(ClassDefinition).getMethod(name) and - not method.isStatic() and - not method.isAmbient() and - not method instanceof ConstructorDeclaration and - result = method.getBody().flow() - ) - or - result = getAPrototypeReference().getAPropertyWrite(name).getRhs().getALocalSource() - } + FunctionNode getInstanceMethod(string name) { result = impl.getInstanceMethod(name) } /** * Gets an instance method of this class. * * The constructor is not considered an instance method. */ - FunctionNode getAnInstanceMethod() { - exists(MethodDeclaration method | - method = astNode.(ClassDefinition).getAMethod() and - not method.isStatic() and - not method.isAmbient() and - not method instanceof ConstructorDeclaration and - result = method.getBody().flow() - ) - or - result = getAPrototypeReference().getAPropertyWrite().getRhs().getALocalSource() - } + FunctionNode getAnInstanceMethod() { result = impl.getAnInstanceMethod() } /** * Gets the static method of this class with the given name. */ - FunctionNode getStaticMethod(string name) { - exists(MethodDeclaration method | - method = astNode.(ClassDefinition).getMethod(name) and - method.isStatic() and - not method.isAmbient() and - result = method.getBody().flow() - ) - or - result = getAPropertyWrite(name).getRhs().getALocalSource() - } + FunctionNode getStaticMethod(string name) { result = impl.getStaticMethod(name) } /** * Gets a static method of this class. * * The constructor is not considered a static method. */ - FunctionNode getAStaticMethod() { - exists(MethodDeclaration method | - method = astNode.(ClassDefinition).getAMethod() and - method.isStatic() and - not method.isAmbient() and - result = method.getBody().flow() - ) - or - result = getAPropertyWrite().getRhs().getALocalSource() - } + FunctionNode getAStaticMethod() { result = impl.getAStaticMethod() } +} - /** - * Gets a reference to the prototype of this class. - */ - private DataFlow::SourceNode getAPrototypeReference() { - result = getAPropertyRead("prototype") - or - result = getAPropertySource("prototype") - or - exists(ExtendCall call | - call.getDestinationOperand() = getAPropertyRead("prototype") and - result = call.getASourceOperand() - ) +module ClassNode { + class Range extends DataFlow::SourceNode, DataFlow::ValueNode { + Range() { + astNode instanceof ClassDefinition + or + astNode instanceof Function and + exists(getAPropertyReference("prototype")) + } + + /** + * Gets the name of the class, if it has one. + */ + string getName() { + result = astNode.(ClassDefinition).getName() + or + result = astNode.(Function).getName() + } + + /** + * Gets a description of the class. + */ + string describe() { + result = astNode.(ClassDefinition).describe() + or + result = astNode.(Function).describe() + } + + /** + * Gets the constructor function of this class. + */ + FunctionNode getConstructor() { + result = astNode.(ClassDefinition).getConstructor().getBody().flow() + or + result = this + } + + /** + * Gets an instance method with the given name, if any. + */ + FunctionNode getInstanceMethod(string name) { + exists(MethodDeclaration method | + method = astNode.(ClassDefinition).getMethod(name) and + not method.isStatic() and + not method.isAmbient() and + not method instanceof ConstructorDeclaration and + result = method.getBody().flow() + ) + or + result = getAPrototypeReference().getAPropertyWrite(name).getRhs().getALocalSource() + } + + /** + * Gets an instance method of this class. + * + * The constructor is not considered an instance method. + */ + FunctionNode getAnInstanceMethod() { + exists(MethodDeclaration method | + method = astNode.(ClassDefinition).getAMethod() and + not method.isStatic() and + not method.isAmbient() and + not method instanceof ConstructorDeclaration and + result = method.getBody().flow() + ) + or + result = getAPrototypeReference().getAPropertyWrite().getRhs().getALocalSource() + } + + /** + * Gets the static method of this class with the given name. + */ + FunctionNode getStaticMethod(string name) { + exists(MethodDeclaration method | + method = astNode.(ClassDefinition).getMethod(name) and + method.isStatic() and + not method.isAmbient() and + result = method.getBody().flow() + ) + or + result = getAPropertyWrite(name).getRhs().getALocalSource() + } + + /** + * Gets a static method of this class. + * + * The constructor is not considered a static method. + */ + FunctionNode getAStaticMethod() { + exists(MethodDeclaration method | + method = astNode.(ClassDefinition).getAMethod() and + method.isStatic() and + not method.isAmbient() and + result = method.getBody().flow() + ) + or + result = getAPropertyWrite().getRhs().getALocalSource() + } + + /** + * Gets a reference to the prototype of this class. + */ + private DataFlow::SourceNode getAPrototypeReference() { + result = getAPropertyRead("prototype") + or + result = getAPropertySource("prototype") + or + exists(ExtendCall call | + call.getDestinationOperand() = getAPropertyRead("prototype") and + result = call.getASourceOperand() + ) + } } } From 1eb0ca4b4a7ab6415c59fed2c7db6f5fd644257f Mon Sep 17 00:00:00 2001 From: Asger F Date: Mon, 14 Jan 2019 15:18:41 +0000 Subject: [PATCH 3/7] JS: make ClassNode::Range abstract --- .../src/semmle/javascript/dataflow/Nodes.qll | 131 +++++++++++------- 1 file changed, 82 insertions(+), 49 deletions(-) diff --git a/javascript/ql/src/semmle/javascript/dataflow/Nodes.qll b/javascript/ql/src/semmle/javascript/dataflow/Nodes.qll index e5d8ba939476..ce3e9c319fe5 100644 --- a/javascript/ql/src/semmle/javascript/dataflow/Nodes.qll +++ b/javascript/ql/src/semmle/javascript/dataflow/Nodes.qll @@ -502,7 +502,7 @@ class ClassNode extends DataFlow::SourceNode { /** * Gets a description of the class. */ - string desribe() { result = impl.describe() } + string describe() { result = impl.describe() } /** * Gets the constructor function of this class. @@ -535,79 +535,88 @@ class ClassNode extends DataFlow::SourceNode { } module ClassNode { - class Range extends DataFlow::SourceNode, DataFlow::ValueNode { - Range() { - astNode instanceof ClassDefinition - or - astNode instanceof Function and - exists(getAPropertyReference("prototype")) - } - + /** + * A dataflow node that should be considered a class node. + * + * Subclass this to introduce new kinds of class nodes. If you want to refine + * the definition of existing class nodes, subclass `DataFlow::ClassNode` instead. + */ + abstract class Range extends DataFlow::SourceNode { /** * Gets the name of the class, if it has one. */ - string getName() { - result = astNode.(ClassDefinition).getName() - or - result = astNode.(Function).getName() - } + abstract string getName(); /** * Gets a description of the class. */ - string describe() { - result = astNode.(ClassDefinition).describe() - or - result = astNode.(Function).describe() - } + abstract string describe(); /** * Gets the constructor function of this class. */ - FunctionNode getConstructor() { - result = astNode.(ClassDefinition).getConstructor().getBody().flow() - or - result = this - } + abstract FunctionNode getConstructor(); /** * Gets an instance method with the given name, if any. */ - FunctionNode getInstanceMethod(string name) { + abstract FunctionNode getInstanceMethod(string name); + + /** + * Gets an instance method of this class. + * + * The constructor is not considered an instance method. + */ + abstract FunctionNode getAnInstanceMethod(); + + /** + * Gets the static method of this class with the given name. + */ + abstract FunctionNode getStaticMethod(string name); + + /** + * Gets a static method of this class. + * + * The constructor is not considered a static method. + */ + abstract FunctionNode getAStaticMethod(); + } + + /** + * An ES6 class as a `ClassNode` instance. + */ + private class ES6Class extends Range, DataFlow::ValueNode { + override ClassDefinition astNode; + + override string getName() { result = astNode.getName() } + + override string describe() { result = astNode.describe() } + + override FunctionNode getConstructor() { result = astNode.getConstructor().getBody().flow() } + + override FunctionNode getInstanceMethod(string name) { exists(MethodDeclaration method | - method = astNode.(ClassDefinition).getMethod(name) and + method = astNode.getMethod(name) and not method.isStatic() and not method.isAmbient() and not method instanceof ConstructorDeclaration and result = method.getBody().flow() ) - or - result = getAPrototypeReference().getAPropertyWrite(name).getRhs().getALocalSource() } - /** - * Gets an instance method of this class. - * - * The constructor is not considered an instance method. - */ - FunctionNode getAnInstanceMethod() { + override FunctionNode getAnInstanceMethod() { exists(MethodDeclaration method | - method = astNode.(ClassDefinition).getAMethod() and + method = astNode.getAMethod() and not method.isStatic() and not method.isAmbient() and not method instanceof ConstructorDeclaration and result = method.getBody().flow() ) - or - result = getAPrototypeReference().getAPropertyWrite().getRhs().getALocalSource() } - /** - * Gets the static method of this class with the given name. - */ - FunctionNode getStaticMethod(string name) { + override FunctionNode getStaticMethod(string name) { exists(MethodDeclaration method | - method = astNode.(ClassDefinition).getMethod(name) and + method = astNode.getMethod(name) and method.isStatic() and not method.isAmbient() and result = method.getBody().flow() @@ -616,19 +625,43 @@ module ClassNode { result = getAPropertyWrite(name).getRhs().getALocalSource() } - /** - * Gets a static method of this class. - * - * The constructor is not considered a static method. - */ - FunctionNode getAStaticMethod() { + override FunctionNode getAStaticMethod() { exists(MethodDeclaration method | method = astNode.(ClassDefinition).getAMethod() and method.isStatic() and not method.isAmbient() and result = method.getBody().flow() ) - or + } + } + + /** + * A function definition with prototype manipulation as a `ClassNode` instance. + */ + class FunctionStyleClass extends Range, DataFlow::ValueNode { + override Function astNode; + + FunctionStyleClass() { exists(getAPropertyReference("prototype")) } + + override string getName() { result = astNode.getName() } + + override string describe() { result = astNode.describe() } + + override FunctionNode getConstructor() { result = this } + + override FunctionNode getInstanceMethod(string name) { + result = getAPrototypeReference().getAPropertyWrite(name).getRhs().getALocalSource() + } + + override FunctionNode getAnInstanceMethod() { + result = getAPrototypeReference().getAPropertyWrite().getRhs().getALocalSource() + } + + override FunctionNode getStaticMethod(string name) { + result = getAPropertyWrite(name).getRhs().getALocalSource() + } + + override FunctionNode getAStaticMethod() { result = getAPropertyWrite().getRhs().getALocalSource() } From cc1204acef0a78a4d19216478f6bffce1a878629 Mon Sep 17 00:00:00 2001 From: Asger F Date: Fri, 18 Jan 2019 14:48:06 +0000 Subject: [PATCH 4/7] JS: remove isAmbient() check --- javascript/ql/src/semmle/javascript/dataflow/Nodes.qll | 4 ---- 1 file changed, 4 deletions(-) diff --git a/javascript/ql/src/semmle/javascript/dataflow/Nodes.qll b/javascript/ql/src/semmle/javascript/dataflow/Nodes.qll index ce3e9c319fe5..3ff3c73f96e1 100644 --- a/javascript/ql/src/semmle/javascript/dataflow/Nodes.qll +++ b/javascript/ql/src/semmle/javascript/dataflow/Nodes.qll @@ -598,7 +598,6 @@ module ClassNode { exists(MethodDeclaration method | method = astNode.getMethod(name) and not method.isStatic() and - not method.isAmbient() and not method instanceof ConstructorDeclaration and result = method.getBody().flow() ) @@ -608,7 +607,6 @@ module ClassNode { exists(MethodDeclaration method | method = astNode.getAMethod() and not method.isStatic() and - not method.isAmbient() and not method instanceof ConstructorDeclaration and result = method.getBody().flow() ) @@ -618,7 +616,6 @@ module ClassNode { exists(MethodDeclaration method | method = astNode.getMethod(name) and method.isStatic() and - not method.isAmbient() and result = method.getBody().flow() ) or @@ -629,7 +626,6 @@ module ClassNode { exists(MethodDeclaration method | method = astNode.(ClassDefinition).getAMethod() and method.isStatic() and - not method.isAmbient() and result = method.getBody().flow() ) } From c82690f4c16f3d34b8d03f7227ccdecc15c47f1b Mon Sep 17 00:00:00 2001 From: Asger F Date: Tue, 15 Jan 2019 13:14:10 +0000 Subject: [PATCH 5/7] JS: address comments --- .../src/semmle/javascript/dataflow/Nodes.qll | 22 ++++++++++++------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/javascript/ql/src/semmle/javascript/dataflow/Nodes.qll b/javascript/ql/src/semmle/javascript/dataflow/Nodes.qll index 3ff3c73f96e1..6418b5f490c2 100644 --- a/javascript/ql/src/semmle/javascript/dataflow/Nodes.qll +++ b/javascript/ql/src/semmle/javascript/dataflow/Nodes.qll @@ -488,6 +488,8 @@ DataFlow::SourceNode moduleMember(string path, string m) { * method: function() {} * }); * ``` + * + * Additional patterns can be recognized as class nodes, by extending `DataFlow::ClassNode::Range`. */ class ClassNode extends DataFlow::SourceNode { ClassNode::Range impl; @@ -510,24 +512,28 @@ class ClassNode extends DataFlow::SourceNode { FunctionNode getConstructor() { result = impl.getConstructor() } /** - * Gets an instance method with the given name, if any. + * Gets an instance method declared in this class, with the given name, if any. + * + * Does not include methods from superclasses. */ FunctionNode getInstanceMethod(string name) { result = impl.getInstanceMethod(name) } /** - * Gets an instance method of this class. + * Gets an instance method declared in this class. * * The constructor is not considered an instance method. + * + * Does not include methods from superclasses. */ FunctionNode getAnInstanceMethod() { result = impl.getAnInstanceMethod() } /** - * Gets the static method of this class with the given name. + * Gets the static method declared in this class with the given name. */ FunctionNode getStaticMethod(string name) { result = impl.getStaticMethod(name) } /** - * Gets a static method of this class. + * Gets a static method declared in this class. * * The constructor is not considered a static method. */ @@ -619,12 +625,12 @@ module ClassNode { result = method.getBody().flow() ) or - result = getAPropertyWrite(name).getRhs().getALocalSource() + result = getAPropertySource(name) } override FunctionNode getAStaticMethod() { exists(MethodDeclaration method | - method = astNode.(ClassDefinition).getAMethod() and + method = astNode.getAMethod() and method.isStatic() and result = method.getBody().flow() ) @@ -646,7 +652,7 @@ module ClassNode { override FunctionNode getConstructor() { result = this } override FunctionNode getInstanceMethod(string name) { - result = getAPrototypeReference().getAPropertyWrite(name).getRhs().getALocalSource() + result = getAPrototypeReference().getAPropertySource(name) } override FunctionNode getAnInstanceMethod() { @@ -654,7 +660,7 @@ module ClassNode { } override FunctionNode getStaticMethod(string name) { - result = getAPropertyWrite(name).getRhs().getALocalSource() + result = getAPropertySource(name) } override FunctionNode getAStaticMethod() { From e18b635314317095a195835ff7a81ec7bb45302e Mon Sep 17 00:00:00 2001 From: Asger F Date: Tue, 15 Jan 2019 13:59:41 +0000 Subject: [PATCH 6/7] JS: add getADirectSuperClass() --- .../src/semmle/javascript/dataflow/Nodes.qll | 37 +++++++++++++++++++ .../ClassNode/InstanceMethod.expected | 4 ++ .../library-tests/ClassNode/InstanceMethod.ql | 4 ++ .../ClassNode/SuperClass.expected | 3 ++ .../library-tests/ClassNode/SuperClass.ql | 5 +++ .../ql/test/library-tests/ClassNode/tst.js | 19 ++++++++++ 6 files changed, 72 insertions(+) create mode 100644 javascript/ql/test/library-tests/ClassNode/InstanceMethod.expected create mode 100644 javascript/ql/test/library-tests/ClassNode/InstanceMethod.ql create mode 100644 javascript/ql/test/library-tests/ClassNode/SuperClass.expected create mode 100644 javascript/ql/test/library-tests/ClassNode/SuperClass.ql create mode 100644 javascript/ql/test/library-tests/ClassNode/tst.js diff --git a/javascript/ql/src/semmle/javascript/dataflow/Nodes.qll b/javascript/ql/src/semmle/javascript/dataflow/Nodes.qll index 6418b5f490c2..67034a28d522 100644 --- a/javascript/ql/src/semmle/javascript/dataflow/Nodes.qll +++ b/javascript/ql/src/semmle/javascript/dataflow/Nodes.qll @@ -538,6 +538,18 @@ class ClassNode extends DataFlow::SourceNode { * The constructor is not considered a static method. */ FunctionNode getAStaticMethod() { result = impl.getAStaticMethod() } + + /** + * Gets a direct super class of this class. + */ + ClassNode getADirectSuperClass() { + result.getConstructor().getAstNode() = impl + .getASuperClassNode() + .analyze() + .getAValue() + .(AbstractCallable) + .getFunction() + } } module ClassNode { @@ -586,6 +598,12 @@ module ClassNode { * The constructor is not considered a static method. */ abstract FunctionNode getAStaticMethod(); + + /** + * Gets a dataflow node representing a class to be used as the super-class + * of this node. + */ + abstract DataFlow::Node getASuperClassNode(); } /** @@ -635,6 +653,8 @@ module ClassNode { result = method.getBody().flow() ) } + + override DataFlow::Node getASuperClassNode() { result = astNode.getSuperClass().flow() } } /** @@ -680,5 +700,22 @@ module ClassNode { result = call.getASourceOperand() ) } + + override DataFlow::Node getASuperClassNode() { + // C.prototype = Object.create(D.prototype) + exists(DataFlow::InvokeNode objectCreate, DataFlow::PropRead superProto | + getAPropertySource("prototype") = objectCreate and + objectCreate = DataFlow::globalVarRef("Object").getAMemberCall("create") and + superProto.flowsTo(objectCreate.getArgument(0)) and + superProto.getPropertyName() = "prototype" and + result = superProto.getBase() + ) + or + // C.prototype = new D() + exists(DataFlow::NewNode newCall | + getAPropertySource("prototype") = newCall and + result = newCall.getCalleeNode() + ) + } } } diff --git a/javascript/ql/test/library-tests/ClassNode/InstanceMethod.expected b/javascript/ql/test/library-tests/ClassNode/InstanceMethod.expected new file mode 100644 index 000000000000..ce653c95de0c --- /dev/null +++ b/javascript/ql/test/library-tests/ClassNode/InstanceMethod.expected @@ -0,0 +1,4 @@ +| tst.js:4:17:4:21 | () {} | A.instanceMethod | +| tst.js:7:6:7:10 | () {} | A.bar | +| tst.js:15:19:15:31 | function() {} | B.foo | +| tst.js:19:19:19:31 | function() {} | C.bar | diff --git a/javascript/ql/test/library-tests/ClassNode/InstanceMethod.ql b/javascript/ql/test/library-tests/ClassNode/InstanceMethod.ql new file mode 100644 index 000000000000..05fb953c8e76 --- /dev/null +++ b/javascript/ql/test/library-tests/ClassNode/InstanceMethod.ql @@ -0,0 +1,4 @@ +import javascript + +from DataFlow::ClassNode cls, string name +select cls.getAnInstanceMethod(name), cls.getName() + "." + name diff --git a/javascript/ql/test/library-tests/ClassNode/SuperClass.expected b/javascript/ql/test/library-tests/ClassNode/SuperClass.expected new file mode 100644 index 000000000000..ae719c0a6b09 --- /dev/null +++ b/javascript/ql/test/library-tests/ClassNode/SuperClass.expected @@ -0,0 +1,3 @@ +| tst.js:11:1:11:21 | class A ... ds A {} | A2 | tst.js:3:1:8:1 | class A ... () {}\\n} | A | +| tst.js:13:1:13:15 | function B() {} | B | tst.js:3:1:8:1 | class A ... () {}\\n} | A | +| tst.js:17:1:17:15 | function C() {} | C | tst.js:13:1:13:15 | function B() {} | B | diff --git a/javascript/ql/test/library-tests/ClassNode/SuperClass.ql b/javascript/ql/test/library-tests/ClassNode/SuperClass.ql new file mode 100644 index 000000000000..425153bc9d3f --- /dev/null +++ b/javascript/ql/test/library-tests/ClassNode/SuperClass.ql @@ -0,0 +1,5 @@ +import javascript + +from DataFlow::ClassNode cls, DataFlow::ClassNode sup +where sup = cls.getADirectSuperClass() +select cls, cls.getName(), sup, sup.getName() diff --git a/javascript/ql/test/library-tests/ClassNode/tst.js b/javascript/ql/test/library-tests/ClassNode/tst.js new file mode 100644 index 000000000000..b0c3745e0019 --- /dev/null +++ b/javascript/ql/test/library-tests/ClassNode/tst.js @@ -0,0 +1,19 @@ +import * as dummy from 'dummy'; // treat as module + +class A { + instanceMethod() {} + static staticMethod() {} + + bar() {} +} + + +class A2 extends A {} + +function B() {} +B.prototype = Object.create(A.prototype); +B.prototype.foo = function() {} + +function C() {} +C.prototype = new B(); +C.prototype.bar = function() {} From 4b4daa645f9177013e94c358650ff22c19888c61 Mon Sep 17 00:00:00 2001 From: Asger F Date: Fri, 18 Jan 2019 15:41:39 +0000 Subject: [PATCH 7/7] JS: handle accessors separately --- .../src/semmle/javascript/dataflow/Nodes.qll | 109 +++++++++++++++--- .../ClassNode/InstanceMember.expected | 8 ++ .../library-tests/ClassNode/InstanceMember.ql | 4 + .../ClassNode/InstanceMethod.expected | 5 +- .../library-tests/ClassNode/InstanceMethod.ql | 2 +- .../ClassNode/SuperClass.expected | 6 +- .../ql/test/library-tests/ClassNode/tst.js | 9 ++ .../Classes/ClassNodeInstanceMethod.expected | 1 - 8 files changed, 123 insertions(+), 21 deletions(-) create mode 100644 javascript/ql/test/library-tests/ClassNode/InstanceMember.expected create mode 100644 javascript/ql/test/library-tests/ClassNode/InstanceMember.ql diff --git a/javascript/ql/src/semmle/javascript/dataflow/Nodes.qll b/javascript/ql/src/semmle/javascript/dataflow/Nodes.qll index 67034a28d522..c5866bd7d17d 100644 --- a/javascript/ql/src/semmle/javascript/dataflow/Nodes.qll +++ b/javascript/ql/src/semmle/javascript/dataflow/Nodes.qll @@ -466,6 +466,44 @@ DataFlow::SourceNode moduleMember(string path, string m) { ) } +/** + * The string `method`, `getter`, or `setter`, representing the kind of a function member + * in a class. + * + * Can can be used with `ClassNode.getInstanceMember` to obtain members of a specific kind. + */ +class MemberKind extends string { + MemberKind() { this = "method" or this = "getter" or this = "setter" } +} + +module MemberKind { + /** The kind of a method, such as `m() {}` */ + MemberKind method() { result = "method" } + + /** The kind of a getter accessor, such as `get f() {}`. */ + MemberKind getter() { result = "getter" } + + /** The kind of a setter accessor, such as `set f() {}`. */ + MemberKind setter() { result = "setter" } + + /** The `getter` and `setter` kinds. */ + MemberKind accessor() { result = getter() or result = setter() } + + /** + * Gets the member kind of a given syntactic member declaration in a class. + */ + MemberKind of(MemberDeclaration decl) { + decl instanceof GetterMethodDeclaration and result = getter() + or + decl instanceof SetterMethodDeclaration and result = setter() + or + decl instanceof MethodDeclaration and + not decl instanceof AccessorMethodDeclaration and + not decl instanceof ConstructorDeclaration and + result = method() + } +} + /** * A data flow node corresponding to a class definition or a function definition * acting as a class. @@ -516,7 +554,7 @@ class ClassNode extends DataFlow::SourceNode { * * Does not include methods from superclasses. */ - FunctionNode getInstanceMethod(string name) { result = impl.getInstanceMethod(name) } + FunctionNode getInstanceMethod(string name) { result = impl.getInstanceMember(name, MemberKind::method()) } /** * Gets an instance method declared in this class. @@ -525,7 +563,28 @@ class ClassNode extends DataFlow::SourceNode { * * Does not include methods from superclasses. */ - FunctionNode getAnInstanceMethod() { result = impl.getAnInstanceMethod() } + FunctionNode getAnInstanceMethod() { result = impl.getAnInstanceMember(MemberKind::method()) } + + /** + * Gets the instance method, getter, or setter with the given name and kind. + * + * Does not include members from superclasses. + */ + FunctionNode getInstanceMember(string name, MemberKind kind) { result = impl.getInstanceMember(name, kind) } + + /** + * Gets an instance method, getter, or setter with the given kind. + * + * Does not include members from superclasses. + */ + FunctionNode getAnInstanceMember(MemberKind kind) { result = impl.getAnInstanceMember(kind) } + + /** + * Gets an instance method, getter, or setter declared in this class. + * + * Does not include members from superclasses. + */ + FunctionNode getAnInstanceMember() { result = impl.getAnInstanceMember(_) } /** * Gets the static method declared in this class with the given name. @@ -576,16 +635,14 @@ module ClassNode { abstract FunctionNode getConstructor(); /** - * Gets an instance method with the given name, if any. + * Gets the instance member with the given name and kind. */ - abstract FunctionNode getInstanceMethod(string name); + abstract FunctionNode getInstanceMember(string name, MemberKind kind); /** - * Gets an instance method of this class. - * - * The constructor is not considered an instance method. + * Gets an instance member with the given kind. */ - abstract FunctionNode getAnInstanceMethod(); + abstract FunctionNode getAnInstanceMember(MemberKind kind); /** * Gets the static method of this class with the given name. @@ -618,20 +675,20 @@ module ClassNode { override FunctionNode getConstructor() { result = astNode.getConstructor().getBody().flow() } - override FunctionNode getInstanceMethod(string name) { + override FunctionNode getInstanceMember(string name, MemberKind kind) { exists(MethodDeclaration method | method = astNode.getMethod(name) and not method.isStatic() and - not method instanceof ConstructorDeclaration and + kind = MemberKind::of(method) and result = method.getBody().flow() ) } - override FunctionNode getAnInstanceMethod() { + override FunctionNode getAnInstanceMember(MemberKind kind) { exists(MethodDeclaration method | method = astNode.getAMethod() and not method.isStatic() and - not method instanceof ConstructorDeclaration and + kind = MemberKind::of(method) and result = method.getBody().flow() ) } @@ -671,12 +728,36 @@ module ClassNode { override FunctionNode getConstructor() { result = this } - override FunctionNode getInstanceMethod(string name) { + private PropertyAccessor getAnAccessor(MemberKind kind) { + result.getObjectExpr() = getAPrototypeReference().asExpr() and + ( + kind = MemberKind::getter() and + result instanceof PropertyGetter + or + kind = MemberKind::setter() and + result instanceof PropertySetter + ) + } + + override FunctionNode getInstanceMember(string name, MemberKind kind) { + kind = MemberKind::method() and result = getAPrototypeReference().getAPropertySource(name) + or + exists (PropertyAccessor accessor | + accessor = getAnAccessor(kind) and + accessor.getName() = name and + result = accessor.getInit().flow() + ) } - override FunctionNode getAnInstanceMethod() { + override FunctionNode getAnInstanceMember(MemberKind kind) { + kind = MemberKind::method() and result = getAPrototypeReference().getAPropertyWrite().getRhs().getALocalSource() + or + exists (PropertyAccessor accessor | + accessor = getAnAccessor(kind) and + result = accessor.getInit().flow() + ) } override FunctionNode getStaticMethod(string name) { diff --git a/javascript/ql/test/library-tests/ClassNode/InstanceMember.expected b/javascript/ql/test/library-tests/ClassNode/InstanceMember.expected new file mode 100644 index 000000000000..f38c5f175f87 --- /dev/null +++ b/javascript/ql/test/library-tests/ClassNode/InstanceMember.expected @@ -0,0 +1,8 @@ +| tst.js:4:17:4:21 | () {} | A.instanceMethod | method | +| tst.js:7:6:7:10 | () {} | A.bar | method | +| tst.js:9:10:9:14 | () {} | A.baz | getter | +| tst.js:17:19:17:31 | function() {} | B.foo | method | +| tst.js:21:19:21:31 | function() {} | C.bar | method | +| tst.js:25:13:25:17 | () {} | D.getter | getter | +| tst.js:26:13:26:18 | (x) {} | D.setter | setter | +| tst.js:27:4:27:8 | () {} | D.m | method | diff --git a/javascript/ql/test/library-tests/ClassNode/InstanceMember.ql b/javascript/ql/test/library-tests/ClassNode/InstanceMember.ql new file mode 100644 index 000000000000..25888a7d893e --- /dev/null +++ b/javascript/ql/test/library-tests/ClassNode/InstanceMember.ql @@ -0,0 +1,4 @@ +import javascript + +from DataFlow::ClassNode cls, string name, string kind +select cls.getInstanceMember(name, kind), cls.getName() + "." + name, kind diff --git a/javascript/ql/test/library-tests/ClassNode/InstanceMethod.expected b/javascript/ql/test/library-tests/ClassNode/InstanceMethod.expected index ce653c95de0c..30a291f6c818 100644 --- a/javascript/ql/test/library-tests/ClassNode/InstanceMethod.expected +++ b/javascript/ql/test/library-tests/ClassNode/InstanceMethod.expected @@ -1,4 +1,5 @@ | tst.js:4:17:4:21 | () {} | A.instanceMethod | | tst.js:7:6:7:10 | () {} | A.bar | -| tst.js:15:19:15:31 | function() {} | B.foo | -| tst.js:19:19:19:31 | function() {} | C.bar | +| tst.js:17:19:17:31 | function() {} | B.foo | +| tst.js:21:19:21:31 | function() {} | C.bar | +| tst.js:27:4:27:8 | () {} | D.m | diff --git a/javascript/ql/test/library-tests/ClassNode/InstanceMethod.ql b/javascript/ql/test/library-tests/ClassNode/InstanceMethod.ql index 05fb953c8e76..9617a1dcbb30 100644 --- a/javascript/ql/test/library-tests/ClassNode/InstanceMethod.ql +++ b/javascript/ql/test/library-tests/ClassNode/InstanceMethod.ql @@ -1,4 +1,4 @@ import javascript from DataFlow::ClassNode cls, string name -select cls.getAnInstanceMethod(name), cls.getName() + "." + name +select cls.getInstanceMethod(name), cls.getName() + "." + name diff --git a/javascript/ql/test/library-tests/ClassNode/SuperClass.expected b/javascript/ql/test/library-tests/ClassNode/SuperClass.expected index ae719c0a6b09..bb35f1ee7119 100644 --- a/javascript/ql/test/library-tests/ClassNode/SuperClass.expected +++ b/javascript/ql/test/library-tests/ClassNode/SuperClass.expected @@ -1,3 +1,3 @@ -| tst.js:11:1:11:21 | class A ... ds A {} | A2 | tst.js:3:1:8:1 | class A ... () {}\\n} | A | -| tst.js:13:1:13:15 | function B() {} | B | tst.js:3:1:8:1 | class A ... () {}\\n} | A | -| tst.js:17:1:17:15 | function C() {} | C | tst.js:13:1:13:15 | function B() {} | B | +| tst.js:13:1:13:21 | class A ... ds A {} | A2 | tst.js:3:1:10:1 | class A ... () {}\\n} | A | +| tst.js:15:1:15:15 | function B() {} | B | tst.js:3:1:10:1 | class A ... () {}\\n} | A | +| tst.js:19:1:19:15 | function C() {} | C | tst.js:15:1:15:15 | function B() {} | B | diff --git a/javascript/ql/test/library-tests/ClassNode/tst.js b/javascript/ql/test/library-tests/ClassNode/tst.js index b0c3745e0019..1774d9afabde 100644 --- a/javascript/ql/test/library-tests/ClassNode/tst.js +++ b/javascript/ql/test/library-tests/ClassNode/tst.js @@ -5,6 +5,8 @@ class A { static staticMethod() {} bar() {} + + get baz() {} } @@ -17,3 +19,10 @@ B.prototype.foo = function() {} function C() {} C.prototype = new B(); C.prototype.bar = function() {} + +function D() {} +D.prototype = { + get getter() {}, + set setter(x) {}, + m() {} +} diff --git a/javascript/ql/test/library-tests/Classes/ClassNodeInstanceMethod.expected b/javascript/ql/test/library-tests/Classes/ClassNodeInstanceMethod.expected index 67429ea3b9d7..aa6a3a2e3cf1 100644 --- a/javascript/ql/test/library-tests/Classes/ClassNodeInstanceMethod.expected +++ b/javascript/ql/test/library-tests/Classes/ClassNodeInstanceMethod.expected @@ -1,4 +1,3 @@ -| points.js:1:1:18:1 | class P ... ;\\n }\\n} | dist | points.js:7:11:9:3 | () {\\n ... y);\\n } | | points.js:1:1:18:1 | class P ... ;\\n }\\n} | toString | points.js:11:11:13:3 | () {\\n ... )";\\n } | | points.js:20:1:33:1 | class C ... ;\\n }\\n} | toString | points.js:26:11:28:3 | () {\\n ... ur;\\n } | | tst.js:1:9:4:1 | class { ... */ }\\n} | constructor | tst.js:3:18:3:56 | () { /* ... r. */ } |