Skip to content

JavaScript: Restructure implementation of DataFlow::SourceNode. #727

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 9, 2019
Merged

JavaScript: Restructure implementation of DataFlow::SourceNode. #727

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions change-notes/1.20/analysis-javascript.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,3 +33,6 @@
| Uncontrolled data used in path expression | Fewer false-positive results | This rule now recognizes the Express `root` option, which prevents path traversal. |

## Changes to QL libraries

* `DataFlow::SourceNode` is no longer an abstract class; to add new source nodes, extend `DataFlow::SourceNode::Range` instead.
* Subclasses of `DataFlow::PropRead` are no longer automatically made source nodes; you now need to additionally define a corresponding subclass of `DataFlow::SourceNode::Range` to achieve this.
2 changes: 1 addition & 1 deletion javascript/ql/src/semmle/javascript/EmailClients.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import javascript
/**
* An operation that sends an email.
*/
abstract class EmailSender extends DataFlow::DefaultSourceNode {
abstract class EmailSender extends DataFlow::SourceNode {
/**
* Gets a data flow node holding the plaintext version of the email body.
*/
Expand Down
2 changes: 1 addition & 1 deletion javascript/ql/src/semmle/javascript/StandardLibrary.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ private class AnalyzedThisInArrayIterationFunction extends AnalyzedNode, DataFlo
/**
* A definition of a `Promise` object.
*/
abstract class PromiseDefinition extends DataFlow::DefaultSourceNode {
abstract class PromiseDefinition extends DataFlow::SourceNode {
/** Gets the executor function of this promise object. */
abstract DataFlow::FunctionNode getExecutor();

Expand Down
14 changes: 14 additions & 0 deletions javascript/ql/src/semmle/javascript/dataflow/DataFlow.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -515,6 +515,20 @@ module DataFlow {
*/
abstract class PropRead extends PropRef, SourceNode { }

/**
* A property read, considered as a source node.
*
* Note that we cannot simplify the characteristic predicate to `this instanceof PropRead`,
* since `PropRead` is itself a subclass of `SourceNode`.
*/
private class PropReadAsSourceNode extends SourceNode::Range {
PropReadAsSourceNode() {
this = TPropNode(any(PropertyPattern p)) or
this instanceof RestPatternNode or
this instanceof ElementPatternNode
}
}

/**
* A property access in rvalue position.
*/
Expand Down
23 changes: 13 additions & 10 deletions javascript/ql/src/semmle/javascript/dataflow/Nodes.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import javascript

/** A data flow node corresponding to a parameter. */
class ParameterNode extends DataFlow::DefaultSourceNode {
class ParameterNode extends DataFlow::SourceNode {
Parameter p;

ParameterNode() { DataFlow::parameterNode(this, p) }
Expand All @@ -20,7 +20,7 @@ class ParameterNode extends DataFlow::DefaultSourceNode {
}

/** A data flow node corresponding to a function invocation (with or without `new`). */
class InvokeNode extends DataFlow::DefaultSourceNode {
class InvokeNode extends DataFlow::SourceNode {
DataFlow::Impl::InvokeNodeDef impl;

InvokeNode() { this = impl }
Expand Down Expand Up @@ -168,7 +168,7 @@ class MethodCallNode extends CallNode {
class NewNode extends InvokeNode { override DataFlow::Impl::NewNodeDef impl; }

/** A data flow node corresponding to the `this` parameter in a function or `this` at the top-level. */
class ThisNode extends DataFlow::Node, DataFlow::DefaultSourceNode {
class ThisNode extends DataFlow::Node, DataFlow::SourceNode {
ThisNode() { DataFlow::thisNode(this, _) }

/**
Expand Down Expand Up @@ -202,7 +202,7 @@ class ThisNode extends DataFlow::Node, DataFlow::DefaultSourceNode {
}

/** A data flow node corresponding to a global variable access. */
class GlobalVarRefNode extends DataFlow::ValueNode, DataFlow::DefaultSourceNode {
class GlobalVarRefNode extends DataFlow::ValueNode, DataFlow::SourceNode {
override GlobalVarAccess astNode;

/** Gets the name of the global variable. */
Expand All @@ -216,7 +216,10 @@ class GlobalVarRefNode extends DataFlow::ValueNode, DataFlow::DefaultSourceNode
*/
DataFlow::SourceNode globalObjectRef() {
// top-level `this`
exists(ThisNode globalThis | result = globalThis | not exists(globalThis.getBinder()))
exists(StmtContainer sc |
sc = result.(ThisNode).getBindingContainer() and
not sc instanceof Function
)
or
// DOM
result = globalVarRef("window")
Expand Down Expand Up @@ -244,7 +247,7 @@ DataFlow::SourceNode globalVarRef(string name) {
}

/** A data flow node corresponding to a function definition. */
class FunctionNode extends DataFlow::ValueNode, DataFlow::DefaultSourceNode {
class FunctionNode extends DataFlow::ValueNode, DataFlow::SourceNode {
override Function astNode;

/** Gets the `i`th parameter of this function. */
Expand Down Expand Up @@ -287,12 +290,12 @@ class FunctionNode extends DataFlow::ValueNode, DataFlow::DefaultSourceNode {
}

/** A data flow node corresponding to an object literal expression. */
class ObjectLiteralNode extends DataFlow::ValueNode, DataFlow::DefaultSourceNode {
class ObjectLiteralNode extends DataFlow::ValueNode, DataFlow::SourceNode {
override ObjectExpr astNode;
}

/** A data flow node corresponding to an array literal expression. */
class ArrayLiteralNode extends DataFlow::ValueNode, DataFlow::DefaultSourceNode {
class ArrayLiteralNode extends DataFlow::ValueNode, DataFlow::SourceNode {
override ArrayExpr astNode;

/** Gets the `i`th element of this array literal. */
Expand Down Expand Up @@ -323,7 +326,7 @@ class ArrayConstructorInvokeNode extends DataFlow::InvokeNode {
* A data flow node corresponding to the creation or a new array, either through an array literal
* or an invocation of the `Array` constructor.
*/
class ArrayCreationNode extends DataFlow::ValueNode, DataFlow::DefaultSourceNode {
class ArrayCreationNode extends DataFlow::ValueNode, DataFlow::SourceNode {
ArrayCreationNode() {
this instanceof ArrayLiteralNode or
this instanceof ArrayConstructorInvokeNode
Expand All @@ -346,7 +349,7 @@ class ArrayCreationNode extends DataFlow::ValueNode, DataFlow::DefaultSourceNode
* For compatibility with old transpilers, we treat `import * from '...'`
* as a default import as well.
*/
class ModuleImportNode extends DataFlow::DefaultSourceNode {
class ModuleImportNode extends DataFlow::SourceNode {
string path;

ModuleImportNode() {
Expand Down
111 changes: 64 additions & 47 deletions javascript/ql/src/semmle/javascript/dataflow/Sources.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,15 @@
import javascript

/**
* A source node for local data flow, that is, a node for which local
* data flow cannot provide any information about its inputs.
* A source node for local data flow, that is, a node from which local data flow is tracked.
*
* By default, functions, object and array expressions and JSX nodes
* are considered sources, as well as expressions that have non-local
* flow (such as calls and property accesses). Additional sources
* can be modelled by extending this class with additional subclasses.
* Examples include function parameters, imports and property accesses; see
* `DataFlow::SourceNode::DefaultRange` for details. You can introduce new kinds of
* source nodes by defining new subclasses of `DataFlow::SourceNode::Range`.
*/
abstract class SourceNode extends DataFlow::Node {
class SourceNode extends DataFlow::Node {
SourceNode() { this instanceof SourceNode::Range }

/**
* Holds if this node flows into `sink` in zero or more local (that is,
* intra-procedural) steps.
Expand Down Expand Up @@ -167,45 +167,62 @@ abstract class SourceNode extends DataFlow::Node {
}
}

/**
* A data flow node that is considered a source node by default.
*
* Currently, the following nodes are source nodes:
* - import specifiers
* - non-destructuring function parameters
* - property accesses
* - function invocations
* - `this` expressions
* - global variable accesses
* - function definitions
* - class definitions
* - object expressions
* - array expressions
* - JSX literals.
*/
class DefaultSourceNode extends SourceNode {
DefaultSourceNode() {
exists(ASTNode astNode | this = DataFlow::valueNode(astNode) |
astNode instanceof PropAccess or
astNode instanceof Function or
astNode instanceof ClassDefinition or
astNode instanceof ObjectExpr or
astNode instanceof ArrayExpr or
astNode instanceof JSXNode or
astNode instanceof GlobalVarAccess or
astNode instanceof ExternalModuleReference
)
or
exists(SsaExplicitDefinition ssa, VarDef def |
this = DataFlow::ssaDefinitionNode(ssa) and def = ssa.getDef()
|
def instanceof ImportSpecifier
)
or
DataFlow::parameterNode(this, _)
or
this instanceof DataFlow::Impl::InvokeNodeDef
or
DataFlow::thisNode(this, _)
module SourceNode {
/**
* A data flow node that should be considered a source node.
*
* Subclass this class to introduce new kinds of source nodes. If you want to refine
* the definition of existing source nodes, subclass `DataFlow::SourceNode` instead.
*/
cached
abstract class Range extends DataFlow::Node { }

/**
* A data flow node that is considered a source node by default.
*
* Currently, the following nodes are source nodes:
* - import specifiers
* - function parameters
* - `this` nodes
* - property accesses
* - function invocations
* - global variable accesses
* - function definitions
* - class definitions
* - object expressions
* - array expressions
* - JSX literals
*
* This class is for internal use only and should not normally be used directly.
*/
class DefaultRange extends Range {
DefaultRange() {
exists(ASTNode astNode | this = DataFlow::valueNode(astNode) |
astNode instanceof PropAccess or
astNode instanceof Function or
astNode instanceof ClassDefinition or
astNode instanceof ObjectExpr or
astNode instanceof ArrayExpr or
astNode instanceof JSXNode or
astNode instanceof GlobalVarAccess or
astNode instanceof ExternalModuleReference
)
or
exists(SsaExplicitDefinition ssa, VarDef def |
this = DataFlow::ssaDefinitionNode(ssa) and def = ssa.getDef()
|
def instanceof ImportSpecifier
)
or
DataFlow::parameterNode(this, _)
or
this instanceof DataFlow::Impl::InvokeNodeDef
or
DataFlow::thisNode(this, _)
}
}
}

deprecated class DefaultSourceNode extends SourceNode {
DefaultSourceNode() { this instanceof SourceNode::DefaultRange }
}
2 changes: 1 addition & 1 deletion javascript/ql/src/semmle/javascript/frameworks/AngularJS/AngularJSCore.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ private predicate isAngularString(Expr s) {
* String literals in Angular code are often used as identifiers or references, so we
* want to track them.
*/
private class TrackStringsInAngularCode extends DataFlow::SourceNode, DataFlow::ValueNode {
private class TrackStringsInAngularCode extends DataFlow::SourceNode::Range, DataFlow::ValueNode {
TrackStringsInAngularCode() { isAngularString(astNode) }
}

Expand Down
4 changes: 4 additions & 0 deletions javascript/ql/src/semmle/javascript/frameworks/HTTP.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -435,6 +435,10 @@ module HTTP {
abstract DataFlow::Node getASecretKey();
}

private class CookieMiddlewareInstanceAsSourceNode extends DataFlow::SourceNode::Range {
CookieMiddlewareInstanceAsSourceNode() { this instanceof CookieMiddlewareInstance }
}

/**
* A key used for signed cookies, viewed as a `CryptographicKey`.
*/
Expand Down
4 changes: 4 additions & 0 deletions javascript/ql/src/semmle/javascript/frameworks/LodashUnderscore.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ module LodashUnderscore {
abstract string getName();
}

private class MemberAsSourceNode extends DataFlow::SourceNode::Range {
MemberAsSourceNode() { this instanceof Member }
}

/**
* An import of `lodash` or `underscore` accessing a given member of that package.
*/
Expand Down
2 changes: 1 addition & 1 deletion javascript/ql/src/semmle/javascript/frameworks/ReactNative.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import javascript

module ReactNative {
/** A `WebView` JSX element. */
class WebViewElement extends DataFlow::ValueNode, DataFlow::DefaultSourceNode {
class WebViewElement extends DataFlow::ValueNode, DataFlow::SourceNode {
override JSXElement astNode;

WebViewElement() {
Expand Down