refactor the SensitiveExpr to be a dataflow node

This commit is contained in:
Erik Krogh Kristensen
2022-04-04 11:14:48 +02:00
committed by erik-krogh
parent 0c4f08c841
commit b4968eb645
9 changed files with 40 additions and 34 deletions

View File

@@ -76,7 +76,7 @@ private predicate canHaveSensitiveCookie(DataFlow::Node node) {
HeuristicNames::nameIndicatesSensitiveData([s, getCookieName(s)], _)
)
or
node.asExpr() instanceof SensitiveExpr
node instanceof SensitiveNode
}
/**

View File

@@ -13,9 +13,25 @@ import javascript
import semmle.javascript.security.internal.SensitiveDataHeuristics
private import HeuristicNames
/**
* DEPRECATED: Use `SensitiveNode` instead.
* An expression that might contain sensitive data.
*/
deprecated class SensitiveExpr extends Expr {
SensitiveNode node;
SensitiveExpr() { node.asExpr() = this }
/** Gets a human-readable description of this expression for use in alert messages. */
deprecated string describe() { result = node.describe() }
/** Gets a classification of the kind of sensitive data this expression might contain. */
deprecated SensitiveDataClassification getClassification() { result = node.getClassification() }
}
/** An expression that might contain sensitive data. */
cached
abstract class SensitiveExpr extends Expr {
abstract class SensitiveNode extends DataFlow::Node {
/** Gets a human-readable description of this expression for use in alert messages. */
cached
abstract string describe();
@@ -26,33 +42,33 @@ abstract class SensitiveExpr extends Expr {
}
/** A function call that might produce sensitive data. */
class SensitiveCall extends SensitiveExpr, InvokeExpr {
class SensitiveCall extends SensitiveNode instanceof DataFlow::InvokeNode {
SensitiveDataClassification classification;
SensitiveCall() {
classification = this.getCalleeName().(SensitiveDataFunctionName).getClassification()
classification = super.getCalleeName().(SensitiveDataFunctionName).getClassification()
or
// This is particularly to pick up methods with an argument like "password", which
// may indicate a lookup.
exists(string s | this.getAnArgument().mayHaveStringValue(s) |
exists(string s | super.getAnArgument().mayHaveStringValue(s) |
nameIndicatesSensitiveData(s, classification)
)
}
override string describe() { result = "a call to " + this.getCalleeName() }
override string describe() { result = "a call to " + super.getCalleeName() }
override SensitiveDataClassification getClassification() { result = classification }
}
/** An access to a variable or property that might contain sensitive data. */
abstract class SensitiveVariableAccess extends SensitiveExpr {
abstract class SensitiveVariableAccess extends SensitiveNode {
string name;
SensitiveVariableAccess() {
this.(VarAccess).getName() = name
this.asExpr().(VarAccess).getName() = name
or
exists(DataFlow::PropRead pr |
this = pr.asExpr() and
this = pr and
pr.getPropertyName() = name
)
}
@@ -173,10 +189,8 @@ class ProtectCall extends DataFlow::CallNode {
}
/** An expression that might contain a clear-text password. */
class CleartextPasswordExpr extends SensitiveExpr {
CleartextPasswordExpr() {
this.(SensitiveExpr).getClassification() = SensitiveDataClassification::password()
}
class CleartextPasswordExpr extends SensitiveNode {
CleartextPasswordExpr() { this.getClassification() = SensitiveDataClassification::password() }
override string describe() { none() }

View File

@@ -30,10 +30,8 @@ module BrokenCryptoAlgorithm {
* A sensitive expression, viewed as a data flow source for sensitive information
* in broken or weak cryptographic algorithms.
*/
class SensitiveExprSource extends Source, DataFlow::ValueNode {
override SensitiveExpr astNode;
override string describe() { result = astNode.describe() }
class SensitiveExprSource extends Source instanceof SensitiveNode {
override string describe() { result = SensitiveNode.super.describe() }
}
/**

View File

@@ -30,15 +30,13 @@ module CleartextStorage {
* A sensitive expression, viewed as a data flow source for cleartext storage
* of sensitive information.
*/
class SensitiveExprSource extends Source, DataFlow::ValueNode {
override SensitiveExpr astNode;
class SensitiveExprSource extends Source instanceof SensitiveNode {
SensitiveExprSource() {
// storing user names or account names in plaintext isn't usually a problem
astNode.getClassification() != SensitiveDataClassification::id()
super.getClassification() != SensitiveDataClassification::id()
}
override string describe() { result = astNode.describe() }
override string describe() { result = SensitiveNode.super.describe() }
}
/** A call to any function whose name suggests that it encodes or encrypts its arguments. */

View File

@@ -30,14 +30,12 @@ module InsufficientPasswordHash {
* A potential clear-text password, considered as a source for password hashing
* with insufficient computational effort.
*/
class CleartextPasswordSource extends Source, DataFlow::ValueNode {
override SensitiveExpr astNode;
class CleartextPasswordSource extends Source instanceof SensitiveNode {
CleartextPasswordSource() {
astNode.getClassification() = SensitiveDataClassification::password()
super.getClassification() = SensitiveDataClassification::password()
}
override string describe() { result = astNode.describe() }
override string describe() { result = SensitiveNode.super.describe() }
}
/**

View File

@@ -41,9 +41,7 @@ module PostMessageStar {
* A sensitive expression, viewed as a data flow source for cross-window communication
* with unrestricted origin.
*/
class SensitiveExprSource extends Source, DataFlow::ValueNode {
override SensitiveExpr astNode;
}
class SensitiveExprSource extends Source instanceof SensitiveNode { }
/** A call to any function whose name suggests that it encodes or encrypts its arguments. */
class ProtectSanitizer extends Sanitizer {

View File

@@ -19,7 +19,7 @@ import DataFlow::PathGraph
from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
where
cfg.hasFlowPath(source, sink) and
not source.getNode().asExpr() instanceof CleartextPasswordExpr // flagged by js/insufficient-password-hash
not source.getNode() instanceof CleartextPasswordExpr // flagged by js/insufficient-password-hash
select sink.getNode(), source, sink,
"Sensitive data from $@ is used in a broken or weak cryptographic algorithm.", source.getNode(),
source.getNode().(Source).describe()

View File

@@ -15,13 +15,13 @@ import javascript
from
Routing::RouteSetup setup, Routing::RouteHandler handler, HTTP::RequestInputAccess input,
SensitiveExpr sensitive
SensitiveNode sensitive
where
setup.getOwnHttpMethod() = "GET" and
setup.getAChild+() = handler and
input.getRouteHandler() = handler.getFunction() and
input.getKind() = "parameter" and
input.(DataFlow::SourceNode).flowsToExpr(sensitive) and
input.(DataFlow::SourceNode).flowsTo(sensitive) and
not sensitive.getClassification() = SensitiveDataClassification::id()
select input, "$@ for GET requests uses query parameter as sensitive data.", handler,
"Route handler"

View File

@@ -20,4 +20,4 @@ query predicate processTermination(NodeJSLib::ProcessTermination term) { any() }
query predicate sensitiveAction(SensitiveAction ac) { any() }
query predicate sensitiveExpr(SensitiveExpr e) { any() }
query predicate sensitiveExpr(SensitiveNode e) { any() }