From f367427fb81cda52249407a5bbcea7201de0e317 Mon Sep 17 00:00:00 2001 From: Anders Schack-Mulligen Date: Mon, 6 May 2019 13:43:58 +0200 Subject: [PATCH] Java: Deprecate RemoteUserInput. --- java/ql/src/Security/CWE/CWE-022/TaintedPath.ql | 2 +- java/ql/src/Security/CWE/CWE-078/ExecCommon.qll | 2 +- java/ql/src/Security/CWE/CWE-079/XSS.ql | 2 +- java/ql/src/Security/CWE/CWE-089/SqlInjectionLib.qll | 2 +- java/ql/src/Security/CWE/CWE-113/ResponseSplitting.ql | 2 +- java/ql/src/Security/CWE/CWE-113/ResponseSplitting.qll | 2 +- .../CWE/CWE-129/ImproperValidationOfArrayConstruction.ql | 2 +- .../CWE/CWE-129/ImproperValidationOfArrayIndex.ql | 2 +- .../CWE/CWE-134/ExternallyControlledFormatString.ql | 2 +- java/ql/src/Security/CWE/CWE-190/ArithmeticTainted.ql | 2 +- java/ql/src/Security/CWE/CWE-502/UnsafeDeserialization.ql | 2 +- java/ql/src/Security/CWE/CWE-601/UrlRedirect.ql | 2 +- java/ql/src/Security/CWE/CWE-611/XXE.ql | 2 +- java/ql/src/Security/CWE/CWE-681/NumericCastTainted.ql | 2 +- java/ql/src/semmle/code/java/dataflow/FlowSources.qll | 8 ++++++-- java/ql/test/library-tests/frameworks/guice/flow.ql | 2 +- 16 files changed, 21 insertions(+), 17 deletions(-) diff --git a/java/ql/src/Security/CWE/CWE-022/TaintedPath.ql b/java/ql/src/Security/CWE/CWE-022/TaintedPath.ql index 0aef239d264..31fb0c5035c 100644 --- a/java/ql/src/Security/CWE/CWE-022/TaintedPath.ql +++ b/java/ql/src/Security/CWE/CWE-022/TaintedPath.ql @@ -20,7 +20,7 @@ import DataFlow::PathGraph class TaintedPathConfig extends TaintTracking::Configuration { TaintedPathConfig() { this = "TaintedPathConfig" } - override predicate isSource(DataFlow::Node source) { source instanceof RemoteUserInput } + override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource } override predicate isSink(DataFlow::Node sink) { exists(Expr e | e = sink.asExpr() | e = any(PathCreation p).getInput() and not guarded(e)) diff --git a/java/ql/src/Security/CWE/CWE-078/ExecCommon.qll b/java/ql/src/Security/CWE/CWE-078/ExecCommon.qll index 00c7fea3e02..4d6bf8c848a 100644 --- a/java/ql/src/Security/CWE/CWE-078/ExecCommon.qll +++ b/java/ql/src/Security/CWE/CWE-078/ExecCommon.qll @@ -6,7 +6,7 @@ private class RemoteUserInputToArgumentToExecFlowConfig extends TaintTracking::C this = "ExecCommon::RemoteUserInputToArgumentToExecFlowConfig" } - override predicate isSource(DataFlow::Node src) { src instanceof RemoteUserInput } + override predicate isSource(DataFlow::Node src) { src instanceof RemoteFlowSource } override predicate isSink(DataFlow::Node sink) { sink.asExpr() instanceof ArgumentToExec } diff --git a/java/ql/src/Security/CWE/CWE-079/XSS.ql b/java/ql/src/Security/CWE/CWE-079/XSS.ql index dc4c7f606bc..5b1bfb2ab16 100644 --- a/java/ql/src/Security/CWE/CWE-079/XSS.ql +++ b/java/ql/src/Security/CWE/CWE-079/XSS.ql @@ -18,7 +18,7 @@ import DataFlow2::PathGraph class XSSConfig extends TaintTracking::Configuration2 { XSSConfig() { this = "XSSConfig" } - override predicate isSource(DataFlow::Node source) { source instanceof RemoteUserInput } + override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource } override predicate isSink(DataFlow::Node sink) { sink instanceof XssSink } diff --git a/java/ql/src/Security/CWE/CWE-089/SqlInjectionLib.qll b/java/ql/src/Security/CWE/CWE-089/SqlInjectionLib.qll index 09da2e6de2a..3d2e2b91434 100644 --- a/java/ql/src/Security/CWE/CWE-089/SqlInjectionLib.qll +++ b/java/ql/src/Security/CWE/CWE-089/SqlInjectionLib.qll @@ -49,7 +49,7 @@ class PersistenceQueryInjectionSink extends QueryInjectionSink { private class QueryInjectionFlowConfig extends TaintTracking::Configuration { QueryInjectionFlowConfig() { this = "SqlInjectionLib::QueryInjectionFlowConfig" } - override predicate isSource(DataFlow::Node src) { src instanceof RemoteUserInput } + override predicate isSource(DataFlow::Node src) { src instanceof RemoteFlowSource } override predicate isSink(DataFlow::Node sink) { sink instanceof QueryInjectionSink } diff --git a/java/ql/src/Security/CWE/CWE-113/ResponseSplitting.ql b/java/ql/src/Security/CWE/CWE-113/ResponseSplitting.ql index c2a6e0363a0..ce5efe56d70 100644 --- a/java/ql/src/Security/CWE/CWE-113/ResponseSplitting.ql +++ b/java/ql/src/Security/CWE/CWE-113/ResponseSplitting.ql @@ -18,7 +18,7 @@ class ResponseSplittingConfig extends TaintTracking::Configuration { ResponseSplittingConfig() { this = "ResponseSplittingConfig" } override predicate isSource(DataFlow::Node source) { - source instanceof RemoteUserInput and + source instanceof RemoteFlowSource and not source instanceof WhitelistedSource } diff --git a/java/ql/src/Security/CWE/CWE-113/ResponseSplitting.qll b/java/ql/src/Security/CWE/CWE-113/ResponseSplitting.qll index f2bcf80d1cc..f17ac91fa97 100644 --- a/java/ql/src/Security/CWE/CWE-113/ResponseSplitting.qll +++ b/java/ql/src/Security/CWE/CWE-113/ResponseSplitting.qll @@ -30,7 +30,7 @@ class HeaderSplittingSink extends DataFlow::ExprNode { } } -class WhitelistedSource extends RemoteUserInput { +class WhitelistedSource extends DataFlow::ExprNode { WhitelistedSource() { this.asExpr().(MethodAccess).getMethod() instanceof HttpServletRequestGetHeaderMethod or this.asExpr().(MethodAccess).getMethod() instanceof CookieGetNameMethod diff --git a/java/ql/src/Security/CWE/CWE-129/ImproperValidationOfArrayConstruction.ql b/java/ql/src/Security/CWE/CWE-129/ImproperValidationOfArrayConstruction.ql index 3d9c6b0e483..b3d9b9f1884 100644 --- a/java/ql/src/Security/CWE/CWE-129/ImproperValidationOfArrayConstruction.ql +++ b/java/ql/src/Security/CWE/CWE-129/ImproperValidationOfArrayConstruction.ql @@ -17,7 +17,7 @@ import DataFlow::PathGraph class Conf extends TaintTracking::Configuration { Conf() { this = "RemoteUserInputTocanThrowOutOfBoundsDueToEmptyArrayConfig" } - override predicate isSource(DataFlow::Node source) { source instanceof RemoteUserInput } + override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource } override predicate isSink(DataFlow::Node sink) { any(CheckableArrayAccess caa).canThrowOutOfBoundsDueToEmptyArray(sink.asExpr(), _) diff --git a/java/ql/src/Security/CWE/CWE-129/ImproperValidationOfArrayIndex.ql b/java/ql/src/Security/CWE/CWE-129/ImproperValidationOfArrayIndex.ql index 32571c7f540..9f0d9fa92a2 100644 --- a/java/ql/src/Security/CWE/CWE-129/ImproperValidationOfArrayIndex.ql +++ b/java/ql/src/Security/CWE/CWE-129/ImproperValidationOfArrayIndex.ql @@ -17,7 +17,7 @@ import DataFlow::PathGraph class Conf extends TaintTracking::Configuration { Conf() { this = "RemoteUserInputTocanThrowOutOfBoundsDueToEmptyArrayConfig" } - override predicate isSource(DataFlow::Node source) { source instanceof RemoteUserInput } + override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource } override predicate isSink(DataFlow::Node sink) { any(CheckableArrayAccess caa).canThrowOutOfBounds(sink.asExpr()) diff --git a/java/ql/src/Security/CWE/CWE-134/ExternallyControlledFormatString.ql b/java/ql/src/Security/CWE/CWE-134/ExternallyControlledFormatString.ql index 4251e275e0d..7a9e3a2baab 100644 --- a/java/ql/src/Security/CWE/CWE-134/ExternallyControlledFormatString.ql +++ b/java/ql/src/Security/CWE/CWE-134/ExternallyControlledFormatString.ql @@ -17,7 +17,7 @@ import DataFlow::PathGraph class ExternallyControlledFormatStringConfig extends TaintTracking::Configuration { ExternallyControlledFormatStringConfig() { this = "ExternallyControlledFormatStringConfig" } - override predicate isSource(DataFlow::Node source) { source instanceof RemoteUserInput } + override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource } override predicate isSink(DataFlow::Node sink) { sink.asExpr() = any(StringFormat formatCall).getFormatArgument() diff --git a/java/ql/src/Security/CWE/CWE-190/ArithmeticTainted.ql b/java/ql/src/Security/CWE/CWE-190/ArithmeticTainted.ql index b607b0fcad5..5219d994e5e 100644 --- a/java/ql/src/Security/CWE/CWE-190/ArithmeticTainted.ql +++ b/java/ql/src/Security/CWE/CWE-190/ArithmeticTainted.ql @@ -32,7 +32,7 @@ predicate sink(ArithExpr exp, VarAccess tainted, string effect) { class RemoteUserInputConfig extends TaintTracking::Configuration { RemoteUserInputConfig() { this = "ArithmeticTainted.ql:RemoteUserInputConfig" } - override predicate isSource(DataFlow::Node source) { source instanceof RemoteUserInput } + override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource } override predicate isSink(DataFlow::Node sink) { sink(_, sink.asExpr(), _) } diff --git a/java/ql/src/Security/CWE/CWE-502/UnsafeDeserialization.ql b/java/ql/src/Security/CWE/CWE-502/UnsafeDeserialization.ql index 9d857203538..18cf624375f 100644 --- a/java/ql/src/Security/CWE/CWE-502/UnsafeDeserialization.ql +++ b/java/ql/src/Security/CWE/CWE-502/UnsafeDeserialization.ql @@ -18,7 +18,7 @@ import DataFlow::PathGraph class UnsafeDeserializationConfig extends TaintTracking::Configuration { UnsafeDeserializationConfig() { this = "UnsafeDeserializationConfig" } - override predicate isSource(DataFlow::Node source) { source instanceof RemoteUserInput } + override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource } override predicate isSink(DataFlow::Node sink) { sink instanceof UnsafeDeserializationSink } } diff --git a/java/ql/src/Security/CWE/CWE-601/UrlRedirect.ql b/java/ql/src/Security/CWE/CWE-601/UrlRedirect.ql index b343ae1e8a8..0249051820e 100644 --- a/java/ql/src/Security/CWE/CWE-601/UrlRedirect.ql +++ b/java/ql/src/Security/CWE/CWE-601/UrlRedirect.ql @@ -18,7 +18,7 @@ import DataFlow::PathGraph class UrlRedirectConfig extends TaintTracking::Configuration { UrlRedirectConfig() { this = "UrlRedirectConfig" } - override predicate isSource(DataFlow::Node source) { source instanceof RemoteUserInput } + override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource } override predicate isSink(DataFlow::Node sink) { sink instanceof UrlRedirectSink } } diff --git a/java/ql/src/Security/CWE/CWE-611/XXE.ql b/java/ql/src/Security/CWE/CWE-611/XXE.ql index fba77e0640a..35764f88bc8 100644 --- a/java/ql/src/Security/CWE/CWE-611/XXE.ql +++ b/java/ql/src/Security/CWE/CWE-611/XXE.ql @@ -40,7 +40,7 @@ class UnsafeXxeSink extends DataFlow::ExprNode { class XxeConfig extends TaintTracking::Configuration { XxeConfig() { this = "XXE.ql::XxeConfig" } - override predicate isSource(DataFlow::Node src) { src instanceof RemoteUserInput } + override predicate isSource(DataFlow::Node src) { src instanceof RemoteFlowSource } override predicate isSink(DataFlow::Node sink) { sink instanceof UnsafeXxeSink } } diff --git a/java/ql/src/Security/CWE/CWE-681/NumericCastTainted.ql b/java/ql/src/Security/CWE/CWE-681/NumericCastTainted.ql index 840d1173070..1f54800a091 100644 --- a/java/ql/src/Security/CWE/CWE-681/NumericCastTainted.ql +++ b/java/ql/src/Security/CWE/CWE-681/NumericCastTainted.ql @@ -19,7 +19,7 @@ import DataFlow::PathGraph private class NumericCastFlowConfig extends TaintTracking::Configuration { NumericCastFlowConfig() { this = "NumericCastTainted::RemoteUserInputToNumericNarrowingCastExpr" } - override predicate isSource(DataFlow::Node src) { src instanceof RemoteUserInput } + override predicate isSource(DataFlow::Node src) { src instanceof RemoteFlowSource } override predicate isSink(DataFlow::Node sink) { sink.asExpr() = any(NumericNarrowingCastExpr cast).getExpr() diff --git a/java/ql/src/semmle/code/java/dataflow/FlowSources.qll b/java/ql/src/semmle/code/java/dataflow/FlowSources.qll index dd993fb32db..9059e9b01cc 100644 --- a/java/ql/src/semmle/code/java/dataflow/FlowSources.qll +++ b/java/ql/src/semmle/code/java/dataflow/FlowSources.qll @@ -143,8 +143,12 @@ private class ThriftIfaceParameterSource extends RemoteFlowSource { /** Class for `tainted` user input. */ abstract class UserInput extends DataFlow::Node { } -/** Input that may be controlled by a remote user. */ -class RemoteUserInput extends UserInput { +/** + * DEPRECATED: Use `RemoteFlowSource` instead. + * + * Input that may be controlled by a remote user. + */ +deprecated class RemoteUserInput extends UserInput { RemoteUserInput() { this instanceof RemoteFlowSource } /** diff --git a/java/ql/test/library-tests/frameworks/guice/flow.ql b/java/ql/test/library-tests/frameworks/guice/flow.ql index d447f92fbee..8afd4f440ad 100644 --- a/java/ql/test/library-tests/frameworks/guice/flow.ql +++ b/java/ql/test/library-tests/frameworks/guice/flow.ql @@ -6,7 +6,7 @@ class Conf extends TaintTracking::Configuration { Conf() { this = "conf" } override predicate isSource(DataFlow::Node src) { - src instanceof RemoteUserInput + src instanceof RemoteFlowSource } override predicate isSink(DataFlow::Node sink) {