Merge pull request #12542 from egregius313/egregius313/refactor-more-queries-to-dataflow-module-api

Java: Refactor more queries to the new DataFlow module API (part 2)
This commit is contained in:
Edward Minnix III
2023-03-21 10:35:29 -04:00
committed by GitHub
13 changed files with 130 additions and 106 deletions

View File

@@ -16,16 +16,13 @@ import semmle.code.java.Expr
import semmle.code.java.dataflow.FlowSources
import semmle.code.java.security.ExternalProcess
import semmle.code.java.security.CommandArguments
import DataFlow::PathGraph
class LocalUserInputToArgumentToExecFlowConfig extends TaintTracking::Configuration {
LocalUserInputToArgumentToExecFlowConfig() { this = "LocalUserInputToArgumentToExecFlowConfig" }
module LocalUserInputToArgumentToExecFlowConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node src) { src instanceof LocalUserInput }
override predicate isSource(DataFlow::Node src) { src instanceof LocalUserInput }
predicate isSink(DataFlow::Node sink) { sink.asExpr() instanceof ArgumentToExec }
override predicate isSink(DataFlow::Node sink) { sink.asExpr() instanceof ArgumentToExec }
override predicate isSanitizer(DataFlow::Node node) {
predicate isBarrier(DataFlow::Node node) {
node.getType() instanceof PrimitiveType
or
node.getType() instanceof BoxedType
@@ -34,9 +31,16 @@ class LocalUserInputToArgumentToExecFlowConfig extends TaintTracking::Configurat
}
}
module LocalUserInputToArgumentToExecFlow =
TaintTracking::Make<LocalUserInputToArgumentToExecFlowConfig>;
import LocalUserInputToArgumentToExecFlow::PathGraph
from
DataFlow::PathNode source, DataFlow::PathNode sink, ArgumentToExec execArg,
LocalUserInputToArgumentToExecFlowConfig conf
where conf.hasFlowPath(source, sink) and sink.getNode().asExpr() = execArg
LocalUserInputToArgumentToExecFlow::PathNode source,
LocalUserInputToArgumentToExecFlow::PathNode sink, ArgumentToExec execArg
where
LocalUserInputToArgumentToExecFlow::hasFlowPath(source, sink) and
sink.getNode().asExpr() = execArg
select execArg, source, sink, "This command line depends on a $@.", source.getNode(),
"user-provided value"

View File

@@ -14,17 +14,18 @@
import java
import semmle.code.java.dataflow.FlowSources
import semmle.code.java.security.XSS
import DataFlow::PathGraph
class XssLocalConfig extends TaintTracking::Configuration {
XssLocalConfig() { this = "XSSLocalConfig" }
module XssLocalConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) { source instanceof LocalUserInput }
override predicate isSource(DataFlow::Node source) { source instanceof LocalUserInput }
override predicate isSink(DataFlow::Node sink) { sink instanceof XssSink }
predicate isSink(DataFlow::Node sink) { sink instanceof XssSink }
}
from DataFlow::PathNode source, DataFlow::PathNode sink, XssLocalConfig conf
where conf.hasFlowPath(source, sink)
module XssLocalFlow = TaintTracking::Make<XssLocalConfig>;
import XssLocalFlow::PathGraph
from XssLocalFlow::PathNode source, XssLocalFlow::PathNode sink
where XssLocalFlow::hasFlowPath(source, sink)
select sink.getNode(), source, sink, "Cross-site scripting vulnerability due to $@.",
source.getNode(), "user-provided value"

View File

@@ -25,28 +25,27 @@ class UncontrolledStringBuilderSource extends DataFlow::ExprNode {
}
}
class UncontrolledStringBuilderSourceFlowConfig extends TaintTracking::Configuration {
UncontrolledStringBuilderSourceFlowConfig() {
this = "SqlConcatenated::UncontrolledStringBuilderSourceFlowConfig"
}
module UncontrolledStringBuilderSourceFlowConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node src) { src instanceof UncontrolledStringBuilderSource }
override predicate isSource(DataFlow::Node src) { src instanceof UncontrolledStringBuilderSource }
predicate isSink(DataFlow::Node sink) { sink instanceof QueryInjectionSink }
override predicate isSink(DataFlow::Node sink) { sink instanceof QueryInjectionSink }
override predicate isSanitizer(DataFlow::Node node) {
predicate isBarrier(DataFlow::Node node) {
node.getType() instanceof PrimitiveType or node.getType() instanceof BoxedType
}
}
module UncontrolledStringBuilderSourceFlow =
TaintTracking::Make<UncontrolledStringBuilderSourceFlowConfig>;
from QueryInjectionSink query, Expr uncontrolled
where
(
builtFromUncontrolledConcat(query.asExpr(), uncontrolled)
or
exists(StringBuilderVar sbv, UncontrolledStringBuilderSourceFlowConfig conf |
exists(StringBuilderVar sbv |
uncontrolledStringBuilderQuery(sbv, uncontrolled) and
conf.hasFlow(DataFlow::exprNode(sbv.getToStringCall()), query)
UncontrolledStringBuilderSourceFlow::hasFlow(DataFlow::exprNode(sbv.getToStringCall()), query)
)
) and
not queryTaintedBy(query, _, _)

View File

@@ -15,26 +15,29 @@
import semmle.code.java.Expr
import semmle.code.java.dataflow.FlowSources
import semmle.code.java.security.SqlInjectionQuery
import DataFlow::PathGraph
class LocalUserInputToQueryInjectionFlowConfig extends TaintTracking::Configuration {
LocalUserInputToQueryInjectionFlowConfig() { this = "LocalUserInputToQueryInjectionFlowConfig" }
module LocalUserInputToQueryInjectionFlowConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node src) { src instanceof LocalUserInput }
override predicate isSource(DataFlow::Node src) { src instanceof LocalUserInput }
predicate isSink(DataFlow::Node sink) { sink instanceof QueryInjectionSink }
override predicate isSink(DataFlow::Node sink) { sink instanceof QueryInjectionSink }
override predicate isSanitizer(DataFlow::Node node) {
predicate isBarrier(DataFlow::Node node) {
node.getType() instanceof PrimitiveType or node.getType() instanceof BoxedType
}
override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
any(AdditionalQueryInjectionTaintStep s).step(node1, node2)
}
}
module LocalUserInputToQueryInjectionFlow =
TaintTracking::Make<LocalUserInputToQueryInjectionFlowConfig>;
import LocalUserInputToQueryInjectionFlow::PathGraph
from
DataFlow::PathNode source, DataFlow::PathNode sink, LocalUserInputToQueryInjectionFlowConfig conf
where conf.hasFlowPath(source, sink)
LocalUserInputToQueryInjectionFlow::PathNode source,
LocalUserInputToQueryInjectionFlow::PathNode sink
where LocalUserInputToQueryInjectionFlow::hasFlowPath(source, sink)
select sink.getNode(), source, sink, "This query depends on a $@.", source.getNode(),
"user-provided value"

View File

@@ -13,25 +13,28 @@
import java
import ArraySizing
import semmle.code.java.dataflow.FlowSources
import DataFlow::PathGraph
class Conf extends TaintTracking::Configuration {
Conf() { this = "RemoteUserInputTocanThrowOutOfBoundsDueToEmptyArrayConfig" }
private module ImproperValidationOfArrayConstructionConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
override predicate isSink(DataFlow::Node sink) {
predicate isSink(DataFlow::Node sink) {
any(CheckableArrayAccess caa).canThrowOutOfBoundsDueToEmptyArray(sink.asExpr(), _)
}
}
module ImproperValidationOfArrayConstructionFlow =
TaintTracking::Make<ImproperValidationOfArrayConstructionConfig>;
import ImproperValidationOfArrayConstructionFlow::PathGraph
from
DataFlow::PathNode source, DataFlow::PathNode sink, Expr sizeExpr,
ImproperValidationOfArrayConstructionFlow::PathNode source,
ImproperValidationOfArrayConstructionFlow::PathNode sink, Expr sizeExpr,
ArrayCreationExpr arrayCreation, CheckableArrayAccess arrayAccess
where
arrayAccess.canThrowOutOfBoundsDueToEmptyArray(sizeExpr, arrayCreation) and
sizeExpr = sink.getNode().asExpr() and
any(Conf conf).hasFlowPath(source, sink)
ImproperValidationOfArrayConstructionFlow::hasFlowPath(source, sink)
select arrayAccess.getIndexExpr(), source, sink,
"This accesses the $@, but the array is initialized using a $@ which may be zero.", arrayCreation,
"array", source.getNode(), "user-provided value"

View File

@@ -13,30 +13,33 @@
import java
import ArraySizing
import DataFlow::PathGraph
import semmle.code.java.dataflow.TaintTracking
class BoundedFlowSourceConf extends DataFlow::Configuration {
BoundedFlowSourceConf() { this = "BoundedFlowSource" }
override predicate isSource(DataFlow::Node source) {
module BoundedFlowSourceConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) {
source instanceof BoundedFlowSource and
// There is not a fixed lower bound which is greater than zero.
not source.(BoundedFlowSource).lowerBound() > 0
}
override predicate isSink(DataFlow::Node sink) {
predicate isSink(DataFlow::Node sink) {
any(CheckableArrayAccess caa).canThrowOutOfBoundsDueToEmptyArray(sink.asExpr(), _)
}
}
module BoundedFlowSourceFlow = DataFlow::Make<BoundedFlowSourceConfig>;
import BoundedFlowSourceFlow::PathGraph
from
DataFlow::PathNode source, DataFlow::PathNode sink, BoundedFlowSource boundedsource,
Expr sizeExpr, ArrayCreationExpr arrayCreation, CheckableArrayAccess arrayAccess
BoundedFlowSourceFlow::PathNode source, BoundedFlowSourceFlow::PathNode sink,
BoundedFlowSource boundedsource, Expr sizeExpr, ArrayCreationExpr arrayCreation,
CheckableArrayAccess arrayAccess
where
arrayAccess.canThrowOutOfBoundsDueToEmptyArray(sizeExpr, arrayCreation) and
sizeExpr = sink.getNode().asExpr() and
boundedsource = source.getNode() and
any(BoundedFlowSourceConf conf).hasFlowPath(source, sink)
BoundedFlowSourceFlow::hasFlowPath(source, sink)
select arrayAccess.getIndexExpr(), source, sink,
"This accesses the $@, but the array is initialized using $@ which may be zero.", arrayCreation,
"array", boundedsource, boundedsource.getDescription().toLowerCase()

View File

@@ -14,25 +14,28 @@
import java
import ArraySizing
import semmle.code.java.dataflow.FlowSources
import DataFlow::PathGraph
class Conf extends TaintTracking::Configuration {
Conf() { this = "LocalUserInputTocanThrowOutOfBoundsDueToEmptyArrayConfig" }
module ImproperValidationOfArrayConstructionLocalConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) { source instanceof LocalUserInput }
override predicate isSource(DataFlow::Node source) { source instanceof LocalUserInput }
override predicate isSink(DataFlow::Node sink) {
predicate isSink(DataFlow::Node sink) {
any(CheckableArrayAccess caa).canThrowOutOfBoundsDueToEmptyArray(sink.asExpr(), _)
}
}
module ImproperValidationOfArrayConstructionLocalFlow =
TaintTracking::Make<ImproperValidationOfArrayConstructionLocalConfig>;
import ImproperValidationOfArrayConstructionLocalFlow::PathGraph
from
DataFlow::PathNode source, DataFlow::PathNode sink, Expr sizeExpr,
ImproperValidationOfArrayConstructionLocalFlow::PathNode source,
ImproperValidationOfArrayConstructionLocalFlow::PathNode sink, Expr sizeExpr,
ArrayCreationExpr arrayCreation, CheckableArrayAccess arrayAccess
where
arrayAccess.canThrowOutOfBoundsDueToEmptyArray(sizeExpr, arrayCreation) and
sizeExpr = sink.getNode().asExpr() and
any(Conf conf).hasFlowPath(source, sink)
ImproperValidationOfArrayConstructionLocalFlow::hasFlowPath(source, sink)
select arrayAccess.getIndexExpr(), source, sink,
"This accesses the $@, but the array is initialized using a $@ which may be zero.", arrayCreation,
"array", source.getNode(), "user-provided value"

View File

@@ -13,24 +13,28 @@
import java
import ArraySizing
import semmle.code.java.dataflow.FlowSources
import DataFlow::PathGraph
class Conf extends TaintTracking::Configuration {
Conf() { this = "RemoteUserInputTocanThrowOutOfBoundsDueToEmptyArrayConfig" }
module ImproperValidationOfArrayIndexConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
override predicate isSink(DataFlow::Node sink) {
predicate isSink(DataFlow::Node sink) {
any(CheckableArrayAccess caa).canThrowOutOfBounds(sink.asExpr())
}
override predicate isSanitizer(DataFlow::Node node) { node.getType() instanceof BooleanType }
predicate isBarrier(DataFlow::Node node) { node.getType() instanceof BooleanType }
}
from DataFlow::PathNode source, DataFlow::PathNode sink, CheckableArrayAccess arrayAccess
module ImproperValidationOfArrayIndexFlow =
TaintTracking::Make<ImproperValidationOfArrayIndexConfig>;
import ImproperValidationOfArrayIndexFlow::PathGraph
from
ImproperValidationOfArrayIndexFlow::PathNode source,
ImproperValidationOfArrayIndexFlow::PathNode sink, CheckableArrayAccess arrayAccess
where
arrayAccess.canThrowOutOfBounds(sink.getNode().asExpr()) and
any(Conf conf).hasFlowPath(source, sink)
ImproperValidationOfArrayIndexFlow::hasFlowPath(source, sink)
select arrayAccess.getIndexExpr(), source, sink,
"This index depends on a $@ which can cause an ArrayIndexOutOfBoundsException.", source.getNode(),
"user-provided value"

View File

@@ -14,25 +14,27 @@
import java
import ArraySizing
import BoundingChecks
import DataFlow::PathGraph
import semmle.code.java.dataflow.TaintTracking
class BoundedFlowSourceConf extends DataFlow::Configuration {
BoundedFlowSourceConf() { this = "BoundedFlowSource" }
module BoundedFlowSourceConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) { source instanceof BoundedFlowSource }
override predicate isSource(DataFlow::Node source) { source instanceof BoundedFlowSource }
override predicate isSink(DataFlow::Node sink) {
predicate isSink(DataFlow::Node sink) {
exists(CheckableArrayAccess arrayAccess | arrayAccess.canThrowOutOfBounds(sink.asExpr()))
}
}
module BoundedFlowSourceFlow = DataFlow::Make<BoundedFlowSourceConfig>;
import BoundedFlowSourceFlow::PathGraph
from
DataFlow::PathNode source, DataFlow::PathNode sink, BoundedFlowSource boundedsource,
CheckableArrayAccess arrayAccess
BoundedFlowSourceFlow::PathNode source, BoundedFlowSourceFlow::PathNode sink,
BoundedFlowSource boundedsource, CheckableArrayAccess arrayAccess
where
arrayAccess.canThrowOutOfBounds(sink.getNode().asExpr()) and
boundedsource = source.getNode() and
any(BoundedFlowSourceConf conf).hasFlowPath(source, sink) and
BoundedFlowSourceFlow::hasFlowPath(source, sink) and
boundedsource != sink.getNode() and
not (
(

View File

@@ -14,22 +14,26 @@
import java
import ArraySizing
import semmle.code.java.dataflow.FlowSources
import DataFlow::PathGraph
class Conf extends TaintTracking::Configuration {
Conf() { this = "LocalUserInputTocanThrowOutOfBoundsDueToEmptyArrayConfig" }
module ImproperValidationOfArrayIndexLocalConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) { source instanceof LocalUserInput }
override predicate isSource(DataFlow::Node source) { source instanceof LocalUserInput }
override predicate isSink(DataFlow::Node sink) {
predicate isSink(DataFlow::Node sink) {
any(CheckableArrayAccess caa).canThrowOutOfBounds(sink.asExpr())
}
}
from DataFlow::PathNode source, DataFlow::PathNode sink, CheckableArrayAccess arrayAccess
module ImproperValidationOfArrayIndexLocalFlow =
TaintTracking::Make<ImproperValidationOfArrayIndexLocalConfig>;
import ImproperValidationOfArrayIndexLocalFlow::PathGraph
from
ImproperValidationOfArrayIndexLocalFlow::PathNode source,
ImproperValidationOfArrayIndexLocalFlow::PathNode sink, CheckableArrayAccess arrayAccess
where
arrayAccess.canThrowOutOfBounds(sink.getNode().asExpr()) and
any(Conf conf).hasFlowPath(source, sink)
ImproperValidationOfArrayIndexLocalFlow::hasFlowPath(source, sink)
select arrayAccess.getIndexExpr(), source, sink,
"This index depends on a $@ which can cause an ArrayIndexOutOfBoundsException.", source.getNode(),
"user-provided value"

View File

@@ -33,7 +33,7 @@ class ExtremeSource extends VarAccess {
ExtremeSource() { this.getVariable() instanceof ExtremeValueField }
}
private module MaxValueFlowConfig implements DataFlow::ConfigSig {
module MaxValueFlowConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) {
source.asExpr().(ExtremeSource).getVariable() instanceof MaxValueField
}
@@ -47,7 +47,7 @@ private module MaxValueFlowConfig implements DataFlow::ConfigSig {
module MaxValueFlow = DataFlow::Make<MaxValueFlowConfig>;
private module MinValueFlowConfig implements DataFlow::ConfigSig {
module MinValueFlowConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) {
source.asExpr().(ExtremeSource).getVariable() instanceof MinValueField
}

View File

@@ -120,22 +120,20 @@ class GetMessageFlowSource extends MethodAccess {
}
}
class GetMessageFlowSourceToHttpResponseSinkFlowConfig extends TaintTracking::Configuration {
GetMessageFlowSourceToHttpResponseSinkFlowConfig() {
this = "StackTraceExposure::GetMessageFlowSourceToHttpResponseSinkFlowConfig"
}
module GetMessageFlowSourceToHttpResponseSinkFlowConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node src) { src.asExpr() instanceof GetMessageFlowSource }
override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof GetMessageFlowSource }
override predicate isSink(DataFlow::Node sink) { sink instanceof InformationLeakSink }
predicate isSink(DataFlow::Node sink) { sink instanceof InformationLeakSink }
}
module GetMessageFlowSourceToHttpResponseSinkFlow =
TaintTracking::Make<GetMessageFlowSourceToHttpResponseSinkFlowConfig>;
/**
* A call to `getMessage()` that then flows to a servlet response.
*/
predicate getMessageFlowsExternally(DataFlow::Node externalExpr, GetMessageFlowSource getMessage) {
any(GetMessageFlowSourceToHttpResponseSinkFlowConfig conf)
.hasFlow(DataFlow::exprNode(getMessage), externalExpr)
GetMessageFlowSourceToHttpResponseSinkFlow::hasFlow(DataFlow::exprNode(getMessage), externalExpr)
}
from Expr externalExpr, Expr errorInformation

View File

@@ -26,10 +26,8 @@ predicate isSafeSecureCookieSetting(Expr e) {
)
}
class SecureCookieConfiguration extends DataFlow::Configuration {
SecureCookieConfiguration() { this = "SecureCookieConfiguration" }
override predicate isSource(DataFlow::Node source) {
module SecureCookieConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) {
exists(MethodAccess ma, Method m | ma.getMethod() = m |
m.getDeclaringType() instanceof TypeCookie and
m.getName() = "setSecure" and
@@ -43,14 +41,16 @@ class SecureCookieConfiguration extends DataFlow::Configuration {
)
}
override predicate isSink(DataFlow::Node sink) {
predicate isSink(DataFlow::Node sink) {
sink.asExpr() =
any(MethodAccess add | add.getMethod() instanceof ResponseAddCookieMethod).getArgument(0)
}
}
module SecureCookieFlow = DataFlow::Make<SecureCookieConfig>;
from MethodAccess add
where
add.getMethod() instanceof ResponseAddCookieMethod and
not any(SecureCookieConfiguration df).hasFlowToExpr(add.getArgument(0))
not SecureCookieFlow::hasFlowToExpr(add.getArgument(0))
select add, "Cookie is added to response without the 'secure' flag being set."