diff --git a/java/ql/lib/semmle/code/java/security/XxeLocalQuery.qll b/java/ql/lib/semmle/code/java/security/XxeLocalQuery.qll index 47d30793f7a..02d68fceb5c 100644 --- a/java/ql/lib/semmle/code/java/security/XxeLocalQuery.qll +++ b/java/ql/lib/semmle/code/java/security/XxeLocalQuery.qll @@ -6,9 +6,11 @@ private import semmle.code.java.dataflow.TaintTracking private import semmle.code.java.security.XxeQuery /** + * DEPRECATED: Use `XxeLocalFlow` instead. + * * A taint-tracking configuration for unvalidated local user input that is used in XML external entity expansion. */ -class XxeLocalConfig extends TaintTracking::Configuration { +deprecated class XxeLocalConfig extends TaintTracking::Configuration { XxeLocalConfig() { this = "XxeLocalConfig" } override predicate isSource(DataFlow::Node src) { src instanceof LocalUserInput } @@ -21,3 +23,23 @@ class XxeLocalConfig extends TaintTracking::Configuration { any(XxeAdditionalTaintStep s).step(n1, n2) } } + +/** + * A taint-tracking configuration for unvalidated local user input that is used in XML external entity expansion. + */ +module XxeLocalConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node src) { src instanceof LocalUserInput } + + predicate isSink(DataFlow::Node sink) { sink instanceof XxeSink } + + predicate isBarrier(DataFlow::Node sanitizer) { sanitizer instanceof XxeSanitizer } + + predicate isAdditionalFlowStep(DataFlow::Node n1, DataFlow::Node n2) { + any(XxeAdditionalTaintStep s).step(n1, n2) + } +} + +/** + * Detect taint flow of unvalidated local user input that is used in XML external entity expansion. + */ +module XxeLocalFlow = TaintTracking::Make; diff --git a/java/ql/lib/semmle/code/java/security/XxeRemoteQuery.qll b/java/ql/lib/semmle/code/java/security/XxeRemoteQuery.qll index f6ace15eba6..46108b7a680 100644 --- a/java/ql/lib/semmle/code/java/security/XxeRemoteQuery.qll +++ b/java/ql/lib/semmle/code/java/security/XxeRemoteQuery.qll @@ -6,9 +6,11 @@ private import semmle.code.java.dataflow.TaintTracking private import semmle.code.java.security.XxeQuery /** + * DEPRECATED: Use `XxeFlow` instead. + * * A taint-tracking configuration for unvalidated remote user input that is used in XML external entity expansion. */ -class XxeConfig extends TaintTracking::Configuration { +deprecated class XxeConfig extends TaintTracking::Configuration { XxeConfig() { this = "XxeConfig" } override predicate isSource(DataFlow::Node src) { src instanceof RemoteFlowSource } @@ -21,3 +23,23 @@ class XxeConfig extends TaintTracking::Configuration { any(XxeAdditionalTaintStep s).step(n1, n2) } } + +/** + * A taint-tracking configuration for unvalidated remote user input that is used in XML external entity expansion. + */ +module XxeConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node src) { src instanceof RemoteFlowSource } + + predicate isSink(DataFlow::Node sink) { sink instanceof XxeSink } + + predicate isBarrier(DataFlow::Node sanitizer) { sanitizer instanceof XxeSanitizer } + + predicate isAdditionalFlowStep(DataFlow::Node n1, DataFlow::Node n2) { + any(XxeAdditionalTaintStep s).step(n1, n2) + } +} + +/** + * Detect taint flow of unvalidated remote user input that is used in XML external entity expansion. + */ +module XxeFlow = TaintTracking::Make; diff --git a/java/ql/src/Security/CWE/CWE-022/TaintedPathLocal.ql b/java/ql/src/Security/CWE/CWE-022/TaintedPathLocal.ql index 506dc2ad65e..fa62e4e6dfd 100644 --- a/java/ql/src/Security/CWE/CWE-022/TaintedPathLocal.ql +++ b/java/ql/src/Security/CWE/CWE-022/TaintedPathLocal.ql @@ -18,32 +18,33 @@ import semmle.code.java.dataflow.FlowSources private import semmle.code.java.dataflow.ExternalFlow import semmle.code.java.security.PathCreation import semmle.code.java.security.PathSanitizer -import DataFlow::PathGraph import TaintedPathCommon -class TaintedPathLocalConfig extends TaintTracking::Configuration { - TaintedPathLocalConfig() { this = "TaintedPathLocalConfig" } +module TaintedPathLocalConfig 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) { sink.asExpr() = any(PathCreation p).getAnInput() or sinkNode(sink, "create-file") } - override predicate isSanitizer(DataFlow::Node sanitizer) { + predicate isBarrier(DataFlow::Node sanitizer) { sanitizer.getType() instanceof BoxedType or sanitizer.getType() instanceof PrimitiveType or sanitizer.getType() instanceof NumberType or sanitizer instanceof PathInjectionSanitizer } - override predicate isAdditionalTaintStep(DataFlow::Node n1, DataFlow::Node n2) { + predicate isAdditionalFlowStep(DataFlow::Node n1, DataFlow::Node n2) { any(TaintedPathAdditionalTaintStep s).step(n1, n2) } } +module TaintedPathLocalFlow = TaintTracking::Make; + +import TaintedPathLocalFlow::PathGraph + /** * Gets the data-flow node at which to report a path ending at `sink`. * @@ -52,13 +53,13 @@ class TaintedPathLocalConfig extends TaintTracking::Configuration { * continue to report there; otherwise we report directly at `sink`. */ DataFlow::Node getReportingNode(DataFlow::Node sink) { - any(TaintedPathLocalConfig c).hasFlowTo(sink) and + TaintedPathLocalFlow::hasFlowTo(sink) and if exists(PathCreation pc | pc.getAnInput() = sink.asExpr()) then result.asExpr() = any(PathCreation pc | pc.getAnInput() = sink.asExpr()) else result = sink } -from DataFlow::PathNode source, DataFlow::PathNode sink, TaintedPathLocalConfig conf -where conf.hasFlowPath(source, sink) +from TaintedPathLocalFlow::PathNode source, TaintedPathLocalFlow::PathNode sink +where TaintedPathLocalFlow::hasFlowPath(source, sink) select getReportingNode(sink.getNode()), source, sink, "This path depends on a $@.", source.getNode(), "user-provided value" diff --git a/java/ql/src/Security/CWE/CWE-022/ZipSlip.ql b/java/ql/src/Security/CWE/CWE-022/ZipSlip.ql index 3f63123cfb6..bae6311f362 100644 --- a/java/ql/src/Security/CWE/CWE-022/ZipSlip.ql +++ b/java/ql/src/Security/CWE/CWE-022/ZipSlip.ql @@ -17,8 +17,6 @@ import semmle.code.java.controlflow.Guards import semmle.code.java.dataflow.SSA import semmle.code.java.dataflow.TaintTracking import semmle.code.java.security.PathSanitizer -import DataFlow -import PathGraph private import semmle.code.java.dataflow.ExternalFlow /** @@ -36,18 +34,20 @@ class ArchiveEntryNameMethod extends Method { } } -class ZipSlipConfiguration extends TaintTracking::Configuration { - ZipSlipConfiguration() { this = "ZipSlip" } - - override predicate isSource(Node source) { +module ZipSlipConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source.asExpr().(MethodAccess).getMethod() instanceof ArchiveEntryNameMethod } - override predicate isSink(Node sink) { sink instanceof FileCreationSink } + predicate isSink(DataFlow::Node sink) { sink instanceof FileCreationSink } - override predicate isSanitizer(Node node) { node instanceof PathInjectionSanitizer } + predicate isBarrier(DataFlow::Node node) { node instanceof PathInjectionSanitizer } } +module ZipSlipFlow = TaintTracking::Make; + +import ZipSlipFlow::PathGraph + /** * A sink that represents a file creation, such as a file write, copy or move operation. */ @@ -55,8 +55,8 @@ private class FileCreationSink extends DataFlow::Node { FileCreationSink() { sinkNode(this, "create-file") } } -from PathNode source, PathNode sink -where any(ZipSlipConfiguration c).hasFlowPath(source, sink) +from ZipSlipFlow::PathNode source, ZipSlipFlow::PathNode sink +where ZipSlipFlow::hasFlowPath(source, sink) select source.getNode(), source, sink, "Unsanitized archive entry, which may contain '..', is used in a $@.", sink.getNode(), "file system operation" diff --git a/java/ql/src/Security/CWE/CWE-090/LdapInjection.ql b/java/ql/src/Security/CWE/CWE-090/LdapInjection.ql index b263c7484a6..66371f161ca 100644 --- a/java/ql/src/Security/CWE/CWE-090/LdapInjection.ql +++ b/java/ql/src/Security/CWE/CWE-090/LdapInjection.ql @@ -14,9 +14,9 @@ import java import semmle.code.java.dataflow.FlowSources import LdapInjectionLib -import DataFlow::PathGraph +import LdapInjectionFlow::PathGraph -from DataFlow::PathNode source, DataFlow::PathNode sink, LdapInjectionFlowConfig conf -where conf.hasFlowPath(source, sink) +from LdapInjectionFlow::PathNode source, LdapInjectionFlow::PathNode sink +where LdapInjectionFlow::hasFlowPath(source, sink) select sink.getNode(), source, sink, "This LDAP query depends on a $@.", source.getNode(), "user-provided value" diff --git a/java/ql/src/Security/CWE/CWE-090/LdapInjectionLib.qll b/java/ql/src/Security/CWE/CWE-090/LdapInjectionLib.qll index 2304ec1cd02..d682e4902d8 100644 --- a/java/ql/src/Security/CWE/CWE-090/LdapInjectionLib.qll +++ b/java/ql/src/Security/CWE/CWE-090/LdapInjectionLib.qll @@ -1,21 +1,20 @@ import java import semmle.code.java.dataflow.FlowSources -import DataFlow import semmle.code.java.security.LdapInjection /** * A taint-tracking configuration for unvalidated user input that is used to construct LDAP queries. */ -class LdapInjectionFlowConfig extends TaintTracking::Configuration { - LdapInjectionFlowConfig() { this = "LdapInjectionFlowConfig" } +module LdapInjectionFlowConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource } - override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource } + predicate isSink(DataFlow::Node sink) { sink instanceof LdapInjectionSink } - override predicate isSink(DataFlow::Node sink) { sink instanceof LdapInjectionSink } + predicate isBarrier(DataFlow::Node node) { node instanceof LdapInjectionSanitizer } - override predicate isSanitizer(DataFlow::Node node) { node instanceof LdapInjectionSanitizer } - - override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) { + predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) { any(LdapInjectionAdditionalTaintStep a).step(pred, succ) } } + +module LdapInjectionFlow = TaintTracking::Make; diff --git a/java/ql/src/Security/CWE/CWE-094/InsecureBeanValidation.ql b/java/ql/src/Security/CWE/CWE-094/InsecureBeanValidation.ql index 2f8e11b207b..c3a991ddcd5 100644 --- a/java/ql/src/Security/CWE/CWE-094/InsecureBeanValidation.ql +++ b/java/ql/src/Security/CWE/CWE-094/InsecureBeanValidation.ql @@ -13,7 +13,6 @@ import java import semmle.code.java.dataflow.TaintTracking import semmle.code.java.dataflow.FlowSources -import DataFlow::PathGraph private import semmle.code.java.dataflow.ExternalFlow /** @@ -56,14 +55,16 @@ class SetMessageInterpolatorCall extends MethodAccess { * Taint tracking BeanValidationConfiguration describing the flow of data from user input * to the argument of a method that builds constraint error messages. */ -class BeanValidationConfig extends TaintTracking::Configuration { - BeanValidationConfig() { this = "BeanValidationConfig" } +module BeanValidationConfig 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) { sink instanceof BeanValidationSink } + predicate isSink(DataFlow::Node sink) { sink instanceof BeanValidationSink } } +module BeanValidationFlow = TaintTracking::Make; + +import BeanValidationFlow::PathGraph + /** * A bean validation sink, such as method `buildConstraintViolationWithTemplate` * declared on a subtype of `javax.validation.ConstraintValidatorContext`. @@ -72,13 +73,13 @@ private class BeanValidationSink extends DataFlow::Node { BeanValidationSink() { sinkNode(this, "bean-validation") } } -from BeanValidationConfig cfg, DataFlow::PathNode source, DataFlow::PathNode sink +from BeanValidationFlow::PathNode source, BeanValidationFlow::PathNode sink where ( not exists(SetMessageInterpolatorCall c) or exists(SetMessageInterpolatorCall c | not c.isSafe()) ) and - cfg.hasFlowPath(source, sink) + BeanValidationFlow::hasFlowPath(source, sink) select sink.getNode(), source, sink, "Custom constraint error message contains an unsanitized $@.", source, "user-provided value" diff --git a/java/ql/src/Security/CWE/CWE-134/ExternallyControlledFormatString.ql b/java/ql/src/Security/CWE/CWE-134/ExternallyControlledFormatString.ql index 942c5d25950..da5bc5372a4 100644 --- a/java/ql/src/Security/CWE/CWE-134/ExternallyControlledFormatString.ql +++ b/java/ql/src/Security/CWE/CWE-134/ExternallyControlledFormatString.ql @@ -13,25 +13,29 @@ import java import semmle.code.java.dataflow.FlowSources import semmle.code.java.StringFormat -import DataFlow::PathGraph -class ExternallyControlledFormatStringConfig extends TaintTracking::Configuration { - ExternallyControlledFormatStringConfig() { this = "ExternallyControlledFormatStringConfig" } +module ExternallyControlledFormatStringConfig 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) { sink.asExpr() = any(StringFormat formatCall).getFormatArgument() } - override predicate isSanitizer(DataFlow::Node node) { + predicate isBarrier(DataFlow::Node node) { node.getType() instanceof NumericType or node.getType() instanceof BooleanType } } +module ExternallyControlledFormatStringFlow = + TaintTracking::Make; + +import ExternallyControlledFormatStringFlow::PathGraph + from - DataFlow::PathNode source, DataFlow::PathNode sink, StringFormat formatCall, - ExternallyControlledFormatStringConfig conf -where conf.hasFlowPath(source, sink) and sink.getNode().asExpr() = formatCall.getFormatArgument() + ExternallyControlledFormatStringFlow::PathNode source, + ExternallyControlledFormatStringFlow::PathNode sink, StringFormat formatCall +where + ExternallyControlledFormatStringFlow::hasFlowPath(source, sink) and + sink.getNode().asExpr() = formatCall.getFormatArgument() select formatCall.getFormatArgument(), source, sink, "Format string depends on a $@.", source.getNode(), "user-provided value" diff --git a/java/ql/src/Security/CWE/CWE-134/ExternallyControlledFormatStringLocal.ql b/java/ql/src/Security/CWE/CWE-134/ExternallyControlledFormatStringLocal.ql index 2cd3a0c29da..0300eaea806 100644 --- a/java/ql/src/Security/CWE/CWE-134/ExternallyControlledFormatStringLocal.ql +++ b/java/ql/src/Security/CWE/CWE-134/ExternallyControlledFormatStringLocal.ql @@ -13,23 +13,25 @@ import java import semmle.code.java.dataflow.FlowSources import semmle.code.java.StringFormat -import DataFlow::PathGraph -class ExternallyControlledFormatStringLocalConfig extends TaintTracking::Configuration { - ExternallyControlledFormatStringLocalConfig() { - this = "ExternallyControlledFormatStringLocalConfig" - } +module ExternallyControlledFormatStringLocalConfig 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) { sink.asExpr() = any(StringFormat formatCall).getFormatArgument() } } +module ExternallyControlledFormatStringLocalFlow = + TaintTracking::Make; + +import ExternallyControlledFormatStringLocalFlow::PathGraph + from - DataFlow::PathNode source, DataFlow::PathNode sink, StringFormat formatCall, - ExternallyControlledFormatStringLocalConfig conf -where conf.hasFlowPath(source, sink) and sink.getNode().asExpr() = formatCall.getFormatArgument() + ExternallyControlledFormatStringLocalFlow::PathNode source, + ExternallyControlledFormatStringLocalFlow::PathNode sink, StringFormat formatCall +where + ExternallyControlledFormatStringLocalFlow::hasFlowPath(source, sink) and + sink.getNode().asExpr() = formatCall.getFormatArgument() select formatCall.getFormatArgument(), source, sink, "Format string depends on a $@.", source.getNode(), "user-provided value" diff --git a/java/ql/src/Security/CWE/CWE-190/ArithmeticTaintedLocal.ql b/java/ql/src/Security/CWE/CWE-190/ArithmeticTaintedLocal.ql index 1dc6e8a6d06..5fae695bf56 100644 --- a/java/ql/src/Security/CWE/CWE-190/ArithmeticTaintedLocal.ql +++ b/java/ql/src/Security/CWE/CWE-190/ArithmeticTaintedLocal.ql @@ -15,35 +15,41 @@ import java import semmle.code.java.dataflow.FlowSources import ArithmeticCommon -import DataFlow::PathGraph -class ArithmeticTaintedLocalOverflowConfig extends TaintTracking::Configuration { - ArithmeticTaintedLocalOverflowConfig() { this = "ArithmeticTaintedLocalOverflowConfig" } +module ArithmeticTaintedLocalOverflowConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source instanceof LocalUserInput } - override predicate isSource(DataFlow::Node source) { source instanceof LocalUserInput } + predicate isSink(DataFlow::Node sink) { overflowSink(_, sink.asExpr()) } - override predicate isSink(DataFlow::Node sink) { overflowSink(_, sink.asExpr()) } - - override predicate isSanitizer(DataFlow::Node n) { overflowBarrier(n) } + predicate isBarrier(DataFlow::Node n) { overflowBarrier(n) } } -class ArithmeticTaintedLocalUnderflowConfig extends TaintTracking::Configuration { - ArithmeticTaintedLocalUnderflowConfig() { this = "ArithmeticTaintedLocalUnderflowConfig" } +module ArithmeticTaintedLocalOverflowFlow = + TaintTracking::Make; - override predicate isSource(DataFlow::Node source) { source instanceof LocalUserInput } +module ArithmeticTaintedLocalUnderflowConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source instanceof LocalUserInput } - override predicate isSink(DataFlow::Node sink) { underflowSink(_, sink.asExpr()) } + predicate isSink(DataFlow::Node sink) { underflowSink(_, sink.asExpr()) } - override predicate isSanitizer(DataFlow::Node n) { underflowBarrier(n) } + predicate isBarrier(DataFlow::Node n) { underflowBarrier(n) } } -from DataFlow::PathNode source, DataFlow::PathNode sink, ArithExpr exp, string effect +module ArithmeticTaintedLocalUnderflowFlow = + TaintTracking::Make; + +module Flow = + DataFlow::MergePathGraph; + +import Flow::PathGraph + +from Flow::PathNode source, Flow::PathNode sink, ArithExpr exp, string effect where - any(ArithmeticTaintedLocalOverflowConfig c).hasFlowPath(source, sink) and + ArithmeticTaintedLocalOverflowFlow::hasFlowPath(source.asPathNode1(), sink.asPathNode1()) and overflowSink(exp, sink.getNode().asExpr()) and effect = "overflow" or - any(ArithmeticTaintedLocalUnderflowConfig c).hasFlowPath(source, sink) and + ArithmeticTaintedLocalUnderflowFlow::hasFlowPath(source.asPathNode2(), sink.asPathNode2()) and underflowSink(exp, sink.getNode().asExpr()) and effect = "underflow" select exp, source, sink, diff --git a/java/ql/src/Security/CWE/CWE-190/ArithmeticUncontrolled.ql b/java/ql/src/Security/CWE/CWE-190/ArithmeticUncontrolled.ql index 015abf5831f..636480f7400 100644 --- a/java/ql/src/Security/CWE/CWE-190/ArithmeticUncontrolled.ql +++ b/java/ql/src/Security/CWE/CWE-190/ArithmeticUncontrolled.ql @@ -17,7 +17,6 @@ import semmle.code.java.dataflow.TaintTracking import semmle.code.java.security.RandomQuery import semmle.code.java.security.SecurityTests import ArithmeticCommon -import DataFlow::PathGraph class TaintSource extends DataFlow::ExprNode { TaintSource() { @@ -25,33 +24,40 @@ class TaintSource extends DataFlow::ExprNode { } } -class ArithmeticUncontrolledOverflowConfig extends TaintTracking::Configuration { - ArithmeticUncontrolledOverflowConfig() { this = "ArithmeticUncontrolledOverflowConfig" } +module ArithmeticUncontrolledOverflowConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source instanceof TaintSource } - override predicate isSource(DataFlow::Node source) { source instanceof TaintSource } + predicate isSink(DataFlow::Node sink) { overflowSink(_, sink.asExpr()) } - override predicate isSink(DataFlow::Node sink) { overflowSink(_, sink.asExpr()) } - - override predicate isSanitizer(DataFlow::Node n) { overflowBarrier(n) } + predicate isBarrier(DataFlow::Node n) { overflowBarrier(n) } } -class ArithmeticUncontrolledUnderflowConfig extends TaintTracking::Configuration { - ArithmeticUncontrolledUnderflowConfig() { this = "ArithmeticUncontrolledUnderflowConfig" } +module ArithmeticUncontrolledOverflowFlow = + TaintTracking::Make; - override predicate isSource(DataFlow::Node source) { source instanceof TaintSource } +module ArithmeticUncontrolledUnderflowConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source instanceof TaintSource } - override predicate isSink(DataFlow::Node sink) { underflowSink(_, sink.asExpr()) } + predicate isSink(DataFlow::Node sink) { underflowSink(_, sink.asExpr()) } - override predicate isSanitizer(DataFlow::Node n) { underflowBarrier(n) } + predicate isBarrier(DataFlow::Node n) { underflowBarrier(n) } } -from DataFlow::PathNode source, DataFlow::PathNode sink, ArithExpr exp, string effect +module ArithmeticUncontrolledUnderflowFlow = + TaintTracking::Make; + +module Flow = + DataFlow::MergePathGraph; + +import Flow::PathGraph + +from Flow::PathNode source, Flow::PathNode sink, ArithExpr exp, string effect where - any(ArithmeticUncontrolledOverflowConfig c).hasFlowPath(source, sink) and + ArithmeticUncontrolledOverflowFlow::hasFlowPath(source.asPathNode1(), sink.asPathNode1()) and overflowSink(exp, sink.getNode().asExpr()) and effect = "overflow" or - any(ArithmeticUncontrolledUnderflowConfig c).hasFlowPath(source, sink) and + ArithmeticUncontrolledUnderflowFlow::hasFlowPath(source.asPathNode2(), sink.asPathNode2()) and underflowSink(exp, sink.getNode().asExpr()) and effect = "underflow" select exp, source, sink, diff --git a/java/ql/src/Security/CWE/CWE-190/ArithmeticWithExtremeValues.ql b/java/ql/src/Security/CWE/CWE-190/ArithmeticWithExtremeValues.ql index cf5cf2f39a8..72801f205aa 100644 --- a/java/ql/src/Security/CWE/CWE-190/ArithmeticWithExtremeValues.ql +++ b/java/ql/src/Security/CWE/CWE-190/ArithmeticWithExtremeValues.ql @@ -16,7 +16,6 @@ import java import semmle.code.java.dataflow.DataFlow import ArithmeticCommon -import DataFlow::PathGraph abstract class ExtremeValueField extends Field { ExtremeValueField() { this.getType() instanceof IntegralType } @@ -34,43 +33,48 @@ class ExtremeSource extends VarAccess { ExtremeSource() { this.getVariable() instanceof ExtremeValueField } } -class MaxValueFlowConfig extends DataFlow::Configuration { - MaxValueFlowConfig() { this = "MaxValueFlowConfig" } - - override predicate isSource(DataFlow::Node source) { +private module MaxValueFlowConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source.asExpr().(ExtremeSource).getVariable() instanceof MaxValueField } - override predicate isSink(DataFlow::Node sink) { overflowSink(_, sink.asExpr()) } + predicate isSink(DataFlow::Node sink) { overflowSink(_, sink.asExpr()) } - override predicate isBarrierIn(DataFlow::Node n) { this.isSource(n) } + predicate isBarrierIn(DataFlow::Node n) { isSource(n) } - override predicate isBarrier(DataFlow::Node n) { overflowBarrier(n) } + predicate isBarrier(DataFlow::Node n) { overflowBarrier(n) } } -class MinValueFlowConfig extends DataFlow::Configuration { - MinValueFlowConfig() { this = "MinValueFlowConfig" } +module MaxValueFlow = DataFlow::Make; - override predicate isSource(DataFlow::Node source) { +private module MinValueFlowConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source.asExpr().(ExtremeSource).getVariable() instanceof MinValueField } - override predicate isSink(DataFlow::Node sink) { underflowSink(_, sink.asExpr()) } + predicate isSink(DataFlow::Node sink) { underflowSink(_, sink.asExpr()) } - override predicate isBarrierIn(DataFlow::Node n) { this.isSource(n) } + predicate isBarrierIn(DataFlow::Node n) { isSource(n) } - override predicate isBarrier(DataFlow::Node n) { underflowBarrier(n) } + predicate isBarrier(DataFlow::Node n) { underflowBarrier(n) } } +module MinValueFlow = DataFlow::Make; + +module Flow = + DataFlow::MergePathGraph; + +import Flow::PathGraph + predicate query( - DataFlow::PathNode source, DataFlow::PathNode sink, ArithExpr exp, string effect, Type srctyp + Flow::PathNode source, Flow::PathNode sink, ArithExpr exp, string effect, Type srctyp ) { ( - any(MaxValueFlowConfig c).hasFlowPath(source, sink) and + MaxValueFlow::hasFlowPath(source.asPathNode1(), sink.asPathNode1()) and overflowSink(exp, sink.getNode().asExpr()) and effect = "overflow" or - any(MinValueFlowConfig c).hasFlowPath(source, sink) and + MinValueFlow::hasFlowPath(source.asPathNode2(), sink.asPathNode2()) and underflowSink(exp, sink.getNode().asExpr()) and effect = "underflow" ) and @@ -78,7 +82,7 @@ predicate query( } from - DataFlow::PathNode source, DataFlow::PathNode sink, ArithExpr exp, Variable v, ExtremeSource s, + Flow::PathNode source, Flow::PathNode sink, ArithExpr exp, Variable v, ExtremeSource s, string effect, Type srctyp where query(source, sink, exp, effect, srctyp) and diff --git a/java/ql/src/Security/CWE/CWE-200/AndroidWebViewSettingsAllowsContentAccess.ql b/java/ql/src/Security/CWE/CWE-200/AndroidWebViewSettingsAllowsContentAccess.ql index d1f4f33917a..1b1ca51e7cd 100644 --- a/java/ql/src/Security/CWE/CWE-200/AndroidWebViewSettingsAllowsContentAccess.ql +++ b/java/ql/src/Security/CWE/CWE-200/AndroidWebViewSettingsAllowsContentAccess.ql @@ -61,10 +61,16 @@ class WebSettingsDisallowContentAccessSink extends DataFlow::Node { } } -class WebViewDisallowContentAccessConfiguration extends TaintTracking::Configuration { - WebViewDisallowContentAccessConfiguration() { this = "WebViewDisallowContentAccessConfiguration" } +private newtype WebViewOrSettings = + IsWebView() or + IsSettings() - override predicate isSource(DataFlow::Node node) { node instanceof WebViewSource } +module WebViewDisallowContentAccessConfig implements DataFlow::StateConfigSig { + class FlowState = WebViewOrSettings; + + predicate isSource(DataFlow::Node node, FlowState state) { + node instanceof WebViewSource and state instanceof IsWebView + } /** * Holds if the step from `node1` to `node2` is a dataflow step that gets the `WebSettings` object @@ -73,12 +79,11 @@ class WebViewDisallowContentAccessConfiguration extends TaintTracking::Configura * This step is only valid when `state1` is empty and `state2` indicates that the `WebSettings` object * has been accessed. */ - override predicate isAdditionalTaintStep( - DataFlow::Node node1, DataFlow::FlowState state1, DataFlow::Node node2, - DataFlow::FlowState state2 + predicate isAdditionalFlowStep( + DataFlow::Node node1, FlowState state1, DataFlow::Node node2, FlowState state2 ) { - state1 instanceof DataFlow::FlowStateEmpty and - state2 = "WebSettings" and + state1 instanceof IsWebView and + state2 instanceof IsSettings and // settings = webView.getSettings() // ^node2 = ^node1 exists(MethodAccess ma | @@ -88,12 +93,17 @@ class WebViewDisallowContentAccessConfiguration extends TaintTracking::Configura ) } - override predicate isSink(DataFlow::Node node, DataFlow::FlowState state) { - state = "WebSettings" and + predicate isSink(DataFlow::Node node, FlowState state) { + state instanceof IsSettings and node instanceof WebSettingsDisallowContentAccessSink } + + predicate isBarrier(DataFlow::Node node, FlowState state) { none() } } +module WebViewDisallowContentAccessFlow = + TaintTracking::MakeWithState; + from Expr e where // explicit: setAllowContentAccess(true) @@ -106,7 +116,7 @@ where // implicit: no setAllowContentAccess(false) exists(WebViewSource source | source.asExpr() = e and - not any(WebViewDisallowContentAccessConfiguration cfg).hasFlow(source, _) + not WebViewDisallowContentAccessFlow::hasFlow(source, _) ) select e, "Sensitive information may be exposed via a malicious link due to access to content:// links being allowed in this WebView." diff --git a/java/ql/src/Security/CWE/CWE-297/UnsafeHostnameVerification.ql b/java/ql/src/Security/CWE/CWE-297/UnsafeHostnameVerification.ql index bc306750e47..f6663b8e87d 100644 --- a/java/ql/src/Security/CWE/CWE-297/UnsafeHostnameVerification.ql +++ b/java/ql/src/Security/CWE/CWE-297/UnsafeHostnameVerification.ql @@ -16,7 +16,6 @@ import semmle.code.java.dataflow.DataFlow import semmle.code.java.dataflow.FlowSources import semmle.code.java.security.Encryption import semmle.code.java.security.SecurityFlag -import DataFlow::PathGraph private import semmle.code.java.dataflow.ExternalFlow /** @@ -45,16 +44,14 @@ class TrustAllHostnameVerifier extends RefType { /** * A configuration to model the flow of a `TrustAllHostnameVerifier` to a `set(Default)HostnameVerifier` call. */ -class TrustAllHostnameVerifierConfiguration extends DataFlow::Configuration { - TrustAllHostnameVerifierConfiguration() { this = "TrustAllHostnameVerifierConfiguration" } - - override predicate isSource(DataFlow::Node source) { +module TrustAllHostnameVerifierConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source.asExpr().(ClassInstanceExpr).getConstructedType() instanceof TrustAllHostnameVerifier } - override predicate isSink(DataFlow::Node sink) { sink instanceof HostnameVerifierSink } + predicate isSink(DataFlow::Node sink) { sink instanceof HostnameVerifierSink } - override predicate isBarrier(DataFlow::Node barrier) { + predicate isBarrier(DataFlow::Node barrier) { // ignore nodes that are in functions that intentionally disable hostname verification barrier .getEnclosingCallable() @@ -80,6 +77,10 @@ class TrustAllHostnameVerifierConfiguration extends DataFlow::Configuration { } } +module TrustAllHostnameVerifierFlow = DataFlow::Make; + +import TrustAllHostnameVerifierFlow::PathGraph + /** * A sink that sets the `HostnameVerifier` on `HttpsURLConnection`. */ @@ -114,10 +115,10 @@ private predicate isNodeGuardedByFlag(DataFlow::Node node) { } from - DataFlow::PathNode source, DataFlow::PathNode sink, TrustAllHostnameVerifierConfiguration cfg, + TrustAllHostnameVerifierFlow::PathNode source, TrustAllHostnameVerifierFlow::PathNode sink, RefType verifier where - cfg.hasFlowPath(source, sink) and + TrustAllHostnameVerifierFlow::hasFlowPath(source, sink) and not isNodeGuardedByFlag(sink.getNode()) and verifier = source.getNode().asExpr().(ClassInstanceExpr).getConstructedType() select sink, source, sink, diff --git a/java/ql/src/Security/CWE/CWE-601/UrlRedirect.ql b/java/ql/src/Security/CWE/CWE-601/UrlRedirect.ql index f3f535e5460..cf9393830b0 100644 --- a/java/ql/src/Security/CWE/CWE-601/UrlRedirect.ql +++ b/java/ql/src/Security/CWE/CWE-601/UrlRedirect.ql @@ -14,17 +14,18 @@ import java import semmle.code.java.dataflow.FlowSources import semmle.code.java.security.UrlRedirect -import DataFlow::PathGraph -class UrlRedirectConfig extends TaintTracking::Configuration { - UrlRedirectConfig() { this = "UrlRedirectConfig" } +module UrlRedirectConfig 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) { sink instanceof UrlRedirectSink } + predicate isSink(DataFlow::Node sink) { sink instanceof UrlRedirectSink } } -from DataFlow::PathNode source, DataFlow::PathNode sink, UrlRedirectConfig conf -where conf.hasFlowPath(source, sink) +module UrlRedirectFlow = TaintTracking::Make; + +import UrlRedirectFlow::PathGraph + +from UrlRedirectFlow::PathNode source, UrlRedirectFlow::PathNode sink +where UrlRedirectFlow::hasFlowPath(source, sink) select sink.getNode(), source, sink, "Untrusted URL redirection depends on a $@.", source.getNode(), "user-provided value" diff --git a/java/ql/src/Security/CWE/CWE-601/UrlRedirectLocal.ql b/java/ql/src/Security/CWE/CWE-601/UrlRedirectLocal.ql index 776cd8cff97..de44173ec57 100644 --- a/java/ql/src/Security/CWE/CWE-601/UrlRedirectLocal.ql +++ b/java/ql/src/Security/CWE/CWE-601/UrlRedirectLocal.ql @@ -14,17 +14,18 @@ import java import semmle.code.java.dataflow.FlowSources import semmle.code.java.security.UrlRedirect -import DataFlow::PathGraph -class UrlRedirectLocalConfig extends TaintTracking::Configuration { - UrlRedirectLocalConfig() { this = "UrlRedirectLocalConfig" } +module UrlRedirectLocalConfig 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 UrlRedirectSink } + predicate isSink(DataFlow::Node sink) { sink instanceof UrlRedirectSink } } -from DataFlow::PathNode source, DataFlow::PathNode sink, UrlRedirectLocalConfig conf -where conf.hasFlowPath(source, sink) +module UrlRedirectLocalFlow = TaintTracking::Make; + +import UrlRedirectLocalFlow::PathGraph + +from UrlRedirectLocalFlow::PathNode source, UrlRedirectLocalFlow::PathNode sink +where UrlRedirectLocalFlow::hasFlowPath(source, sink) select sink.getNode(), source, sink, "Untrusted URL redirection depends on a $@.", source.getNode(), "user-provided value" diff --git a/java/ql/src/Security/CWE/CWE-611/XXE.ql b/java/ql/src/Security/CWE/CWE-611/XXE.ql index af83d7aa35a..708d4f08ee7 100644 --- a/java/ql/src/Security/CWE/CWE-611/XXE.ql +++ b/java/ql/src/Security/CWE/CWE-611/XXE.ql @@ -16,10 +16,10 @@ import java import semmle.code.java.dataflow.DataFlow import semmle.code.java.security.XxeRemoteQuery -import DataFlow::PathGraph +import XxeFlow::PathGraph -from DataFlow::PathNode source, DataFlow::PathNode sink, XxeConfig conf -where conf.hasFlowPath(source, sink) +from XxeFlow::PathNode source, XxeFlow::PathNode sink +where XxeFlow::hasFlowPath(source, sink) select sink.getNode(), source, sink, "XML parsing depends on a $@ without guarding against external entity expansion.", source.getNode(), "user-provided value" diff --git a/java/ql/src/Security/CWE/CWE-611/XXELocal.ql b/java/ql/src/Security/CWE/CWE-611/XXELocal.ql index 6d142921a1d..0ab4ddcc106 100644 --- a/java/ql/src/Security/CWE/CWE-611/XXELocal.ql +++ b/java/ql/src/Security/CWE/CWE-611/XXELocal.ql @@ -16,10 +16,10 @@ import java import semmle.code.java.dataflow.DataFlow import semmle.code.java.security.XxeLocalQuery -import DataFlow::PathGraph +import XxeLocalFlow::PathGraph -from DataFlow::PathNode source, DataFlow::PathNode sink, XxeLocalConfig conf -where conf.hasFlowPath(source, sink) +from XxeLocalFlow::PathNode source, XxeLocalFlow::PathNode sink +where XxeLocalFlow::hasFlowPath(source, sink) select sink.getNode(), source, sink, "XML parsing depends on a $@ without guarding against external entity expansion.", source.getNode(), "user-provided value" diff --git a/java/ql/src/Security/CWE/CWE-643/XPathInjection.ql b/java/ql/src/Security/CWE/CWE-643/XPathInjection.ql index eeaa147ce71..eacebd3f098 100644 --- a/java/ql/src/Security/CWE/CWE-643/XPathInjection.ql +++ b/java/ql/src/Security/CWE/CWE-643/XPathInjection.ql @@ -15,17 +15,18 @@ import java import semmle.code.java.dataflow.FlowSources import semmle.code.java.dataflow.TaintTracking import semmle.code.java.security.XPath -import DataFlow::PathGraph -class XPathInjectionConfiguration extends TaintTracking::Configuration { - XPathInjectionConfiguration() { this = "XPathInjection" } +module XPathInjectionConfig 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) { sink instanceof XPathInjectionSink } + predicate isSink(DataFlow::Node sink) { sink instanceof XPathInjectionSink } } -from DataFlow::PathNode source, DataFlow::PathNode sink, XPathInjectionConfiguration c -where c.hasFlowPath(source, sink) +module XPathInjectionFlow = TaintTracking::Make; + +import XPathInjectionFlow::PathGraph + +from XPathInjectionFlow::PathNode source, XPathInjectionFlow::PathNode sink +where XPathInjectionFlow::hasFlowPath(source, sink) select sink.getNode(), source, sink, "XPath expression depends on a $@.", source.getNode(), "user-provided value" diff --git a/java/ql/src/Security/CWE/CWE-807/TaintedPermissionsCheck.ql b/java/ql/src/Security/CWE/CWE-807/TaintedPermissionsCheck.ql index 1251708ec5f..bb48958443f 100644 --- a/java/ql/src/Security/CWE/CWE-807/TaintedPermissionsCheck.ql +++ b/java/ql/src/Security/CWE/CWE-807/TaintedPermissionsCheck.ql @@ -14,7 +14,7 @@ import java import semmle.code.java.dataflow.FlowSources -import DataFlow::PathGraph +import semmle.code.java.dataflow.TaintTracking class TypeShiroSubject extends RefType { TypeShiroSubject() { this.getQualifiedName() = "org.apache.shiro.subject.Subject" } @@ -52,19 +52,22 @@ class WCPermissionConstruction extends ClassInstanceExpr, PermissionsConstructio override Expr getInput() { result = this.getArgument(0) } } -class TaintedPermissionsCheckFlowConfig extends TaintTracking::Configuration { - TaintedPermissionsCheckFlowConfig() { this = "TaintedPermissionsCheckFlowConfig" } +module TaintedPermissionsCheckFlowConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source instanceof UserInput } - override predicate isSource(DataFlow::Node source) { source instanceof UserInput } - - override predicate isSink(DataFlow::Node sink) { + predicate isSink(DataFlow::Node sink) { sink.asExpr() = any(PermissionsConstruction p).getInput() } } +module TaintedPermissionsCheckFlow = TaintTracking::Make; + +import TaintedPermissionsCheckFlow::PathGraph + from - DataFlow::PathNode source, DataFlow::PathNode sink, PermissionsConstruction p, - TaintedPermissionsCheckFlowConfig conf -where sink.getNode().asExpr() = p.getInput() and conf.hasFlowPath(source, sink) + TaintedPermissionsCheckFlow::PathNode source, TaintedPermissionsCheckFlow::PathNode sink, + PermissionsConstruction p +where + sink.getNode().asExpr() = p.getInput() and TaintedPermissionsCheckFlow::hasFlowPath(source, sink) select p, source, sink, "Permissions check depends on a $@.", source.getNode(), "user-controlled value"