From f5d014baa5666f58ea80538b9ae364626892091d Mon Sep 17 00:00:00 2001 From: Asger F Date: Fri, 1 Mar 2024 21:36:29 +0100 Subject: [PATCH 1/3] JS: Remove allocation site restriction in CG --- .../javascript/dataflow/internal/CallGraphs.qll | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/dataflow/internal/CallGraphs.qll b/javascript/ql/lib/semmle/javascript/dataflow/internal/CallGraphs.qll index d5a5b29d1dc..8ee507f0f98 100644 --- a/javascript/ql/lib/semmle/javascript/dataflow/internal/CallGraphs.qll +++ b/javascript/ql/lib/semmle/javascript/dataflow/internal/CallGraphs.qll @@ -241,13 +241,8 @@ module CallGraph { ) } - private DataFlow::FunctionNode getAMethodOnPlainObject(DataFlow::SourceNode node) { + private DataFlow::FunctionNode getAMethodOnObject(DataFlow::SourceNode node) { ( - ( - node instanceof DataFlow::ObjectLiteralNode - or - node instanceof DataFlow::FunctionNode - ) and result = node.getAPropertySource() or result = node.(DataFlow::ObjectLiteralNode).getPropertyGetter(_) @@ -258,7 +253,7 @@ module CallGraph { } private predicate shouldTrackObjectWithMethods(DataFlow::SourceNode node) { - exists(getAMethodOnPlainObject(node)) + exists(getAMethodOnObject(node)) } /** @@ -292,7 +287,7 @@ module CallGraph { predicate impliedReceiverStep(DataFlow::SourceNode pred, DataFlow::SourceNode succ) { exists(DataFlow::SourceNode host | pred = getAnAllocationSiteRef(host) and - succ = getAMethodOnPlainObject(host).getReceiver() + succ = getAMethodOnObject(host).getReceiver() ) } } From 4ab7acedb69aba3202117a135d570c585a544d00 Mon Sep 17 00:00:00 2001 From: Asger F Date: Mon, 4 Mar 2024 10:32:27 +0100 Subject: [PATCH 2/3] JS: Do not track instance methods --- .../semmle/javascript/dataflow/internal/CallGraphs.qll | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/javascript/ql/lib/semmle/javascript/dataflow/internal/CallGraphs.qll b/javascript/ql/lib/semmle/javascript/dataflow/internal/CallGraphs.qll index 8ee507f0f98..682c9a2dffa 100644 --- a/javascript/ql/lib/semmle/javascript/dataflow/internal/CallGraphs.qll +++ b/javascript/ql/lib/semmle/javascript/dataflow/internal/CallGraphs.qll @@ -249,7 +249,13 @@ module CallGraph { or result = node.(DataFlow::ObjectLiteralNode).getPropertySetter(_) ) and - not node.getTopLevel().isExterns() + not node.getTopLevel().isExterns() and + // Do not track instance methods on classes + not exists(DataFlow::ClassNode cls | + node = cls.getConstructor().getReceiver() + or + node = cls.(DataFlow::ClassNode::FunctionStyleClass).getAPrototypeReference() + ) } private predicate shouldTrackObjectWithMethods(DataFlow::SourceNode node) { From a54a73c9a2d473da215aaa94ad08cd14b816f190 Mon Sep 17 00:00:00 2001 From: Asger F Date: Wed, 6 Mar 2024 11:37:20 +0100 Subject: [PATCH 3/3] JS: Detect more FunctionStyleClasses --- .../ql/lib/semmle/javascript/dataflow/Nodes.qll | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/dataflow/Nodes.qll b/javascript/ql/lib/semmle/javascript/dataflow/Nodes.qll index 192d2e6faa4..d88dab4d431 100644 --- a/javascript/ql/lib/semmle/javascript/dataflow/Nodes.qll +++ b/javascript/ql/lib/semmle/javascript/dataflow/Nodes.qll @@ -1262,6 +1262,12 @@ module ClassNode { result.getFile() = f } + pragma[nomagic] + private DataFlow::NewNode getAnInstantiationInFile(string name, File f) { + result = AccessPath::getAReferenceTo(name).(DataFlow::LocalSourceNode).getAnInstantiation() and + result.getFile() = f + } + /** * Gets a reference to the function `func`, where there exists a read/write of the "prototype" property on that reference. */ @@ -1273,7 +1279,7 @@ module ClassNode { } /** - * A function definition with prototype manipulation as a `ClassNode` instance. + * A function definition, targeted by a `new`-call or with prototype manipulation, seen as a `ClassNode` instance. */ class FunctionStyleClass extends Range, DataFlow::ValueNode { override Function astNode; @@ -1284,9 +1290,12 @@ module ClassNode { ( exists(getAFunctionValueWithPrototype(function)) or - exists(string name | - this = AccessPath::getAnAssignmentTo(name) and + function = any(NewNode new).getCalleeNode().analyze().getAValue() + or + exists(string name | this = AccessPath::getAnAssignmentTo(name) | exists(getAPrototypeReferenceInFile(name, this.getFile())) + or + exists(getAnInstantiationInFile(name, this.getFile())) ) ) }