diff --git a/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImpl1.qll b/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImpl1.qll index 1a8158437bf..2bbc565daa6 100644 --- a/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImpl1.qll +++ b/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImpl1.qll @@ -10,7 +10,7 @@ private import DataFlowImplSpecific::Private import DataFlowImplSpecific::Public private import DataFlowImpl import DataFlowImplCommonPublic -import FlowStateString +deprecated import FlowStateString private import codeql.util.Unit /** diff --git a/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImpl2.qll b/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImpl2.qll index 1a8158437bf..2bbc565daa6 100644 --- a/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImpl2.qll +++ b/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImpl2.qll @@ -10,7 +10,7 @@ private import DataFlowImplSpecific::Private import DataFlowImplSpecific::Public private import DataFlowImpl import DataFlowImplCommonPublic -import FlowStateString +deprecated import FlowStateString private import codeql.util.Unit /** diff --git a/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImpl3.qll b/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImpl3.qll index 1a8158437bf..2bbc565daa6 100644 --- a/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImpl3.qll +++ b/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImpl3.qll @@ -10,7 +10,7 @@ private import DataFlowImplSpecific::Private import DataFlowImplSpecific::Public private import DataFlowImpl import DataFlowImplCommonPublic -import FlowStateString +deprecated import FlowStateString private import codeql.util.Unit /** diff --git a/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImpl4.qll b/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImpl4.qll index 1a8158437bf..2bbc565daa6 100644 --- a/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImpl4.qll +++ b/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImpl4.qll @@ -10,7 +10,7 @@ private import DataFlowImplSpecific::Private import DataFlowImplSpecific::Public private import DataFlowImpl import DataFlowImplCommonPublic -import FlowStateString +deprecated import FlowStateString private import codeql.util.Unit /** diff --git a/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImplLocal.qll b/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImplLocal.qll index 1a8158437bf..2bbc565daa6 100644 --- a/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImplLocal.qll +++ b/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImplLocal.qll @@ -10,7 +10,7 @@ private import DataFlowImplSpecific::Private import DataFlowImplSpecific::Public private import DataFlowImpl import DataFlowImplCommonPublic -import FlowStateString +deprecated import FlowStateString private import codeql.util.Unit /** diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl1.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl1.qll index 1a8158437bf..2bbc565daa6 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl1.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl1.qll @@ -10,7 +10,7 @@ private import DataFlowImplSpecific::Private import DataFlowImplSpecific::Public private import DataFlowImpl import DataFlowImplCommonPublic -import FlowStateString +deprecated import FlowStateString private import codeql.util.Unit /** diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl2.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl2.qll index 1a8158437bf..2bbc565daa6 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl2.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl2.qll @@ -10,7 +10,7 @@ private import DataFlowImplSpecific::Private import DataFlowImplSpecific::Public private import DataFlowImpl import DataFlowImplCommonPublic -import FlowStateString +deprecated import FlowStateString private import codeql.util.Unit /** diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl3.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl3.qll index 1a8158437bf..2bbc565daa6 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl3.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl3.qll @@ -10,7 +10,7 @@ private import DataFlowImplSpecific::Private import DataFlowImplSpecific::Public private import DataFlowImpl import DataFlowImplCommonPublic -import FlowStateString +deprecated import FlowStateString private import codeql.util.Unit /** diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl4.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl4.qll index 1a8158437bf..2bbc565daa6 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl4.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl4.qll @@ -10,7 +10,7 @@ private import DataFlowImplSpecific::Private import DataFlowImplSpecific::Public private import DataFlowImpl import DataFlowImplCommonPublic -import FlowStateString +deprecated import FlowStateString private import codeql.util.Unit /** diff --git a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImpl1.qll b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImpl1.qll index 1a8158437bf..2bbc565daa6 100644 --- a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImpl1.qll +++ b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImpl1.qll @@ -10,7 +10,7 @@ private import DataFlowImplSpecific::Private import DataFlowImplSpecific::Public private import DataFlowImpl import DataFlowImplCommonPublic -import FlowStateString +deprecated import FlowStateString private import codeql.util.Unit /** diff --git a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImpl2.qll b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImpl2.qll index 1a8158437bf..2bbc565daa6 100644 --- a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImpl2.qll +++ b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImpl2.qll @@ -10,7 +10,7 @@ private import DataFlowImplSpecific::Private import DataFlowImplSpecific::Public private import DataFlowImpl import DataFlowImplCommonPublic -import FlowStateString +deprecated import FlowStateString private import codeql.util.Unit /** diff --git a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImpl3.qll b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImpl3.qll index 1a8158437bf..2bbc565daa6 100644 --- a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImpl3.qll +++ b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImpl3.qll @@ -10,7 +10,7 @@ private import DataFlowImplSpecific::Private import DataFlowImplSpecific::Public private import DataFlowImpl import DataFlowImplCommonPublic -import FlowStateString +deprecated import FlowStateString private import codeql.util.Unit /** diff --git a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImpl4.qll b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImpl4.qll index 1a8158437bf..2bbc565daa6 100644 --- a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImpl4.qll +++ b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImpl4.qll @@ -10,7 +10,7 @@ private import DataFlowImplSpecific::Private import DataFlowImplSpecific::Public private import DataFlowImpl import DataFlowImplCommonPublic -import FlowStateString +deprecated import FlowStateString private import codeql.util.Unit /** diff --git a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImpl5.qll b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImpl5.qll index 1a8158437bf..2bbc565daa6 100644 --- a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImpl5.qll +++ b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImpl5.qll @@ -10,7 +10,7 @@ private import DataFlowImplSpecific::Private import DataFlowImplSpecific::Public private import DataFlowImpl import DataFlowImplCommonPublic -import FlowStateString +deprecated import FlowStateString private import codeql.util.Unit /** diff --git a/go/ql/lib/semmle/go/dataflow/internal/DataFlowImpl1.qll b/go/ql/lib/semmle/go/dataflow/internal/DataFlowImpl1.qll index 1a8158437bf..2bbc565daa6 100644 --- a/go/ql/lib/semmle/go/dataflow/internal/DataFlowImpl1.qll +++ b/go/ql/lib/semmle/go/dataflow/internal/DataFlowImpl1.qll @@ -10,7 +10,7 @@ private import DataFlowImplSpecific::Private import DataFlowImplSpecific::Public private import DataFlowImpl import DataFlowImplCommonPublic -import FlowStateString +deprecated import FlowStateString private import codeql.util.Unit /** diff --git a/go/ql/lib/semmle/go/dataflow/internal/DataFlowImpl2.qll b/go/ql/lib/semmle/go/dataflow/internal/DataFlowImpl2.qll index 1a8158437bf..2bbc565daa6 100644 --- a/go/ql/lib/semmle/go/dataflow/internal/DataFlowImpl2.qll +++ b/go/ql/lib/semmle/go/dataflow/internal/DataFlowImpl2.qll @@ -10,7 +10,7 @@ private import DataFlowImplSpecific::Private import DataFlowImplSpecific::Public private import DataFlowImpl import DataFlowImplCommonPublic -import FlowStateString +deprecated import FlowStateString private import codeql.util.Unit /** diff --git a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImpl1.qll b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImpl1.qll index 1a8158437bf..2bbc565daa6 100644 --- a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImpl1.qll +++ b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImpl1.qll @@ -10,7 +10,7 @@ private import DataFlowImplSpecific::Private import DataFlowImplSpecific::Public private import DataFlowImpl import DataFlowImplCommonPublic -import FlowStateString +deprecated import FlowStateString private import codeql.util.Unit /** diff --git a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImpl2.qll b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImpl2.qll index 1a8158437bf..2bbc565daa6 100644 --- a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImpl2.qll +++ b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImpl2.qll @@ -10,7 +10,7 @@ private import DataFlowImplSpecific::Private import DataFlowImplSpecific::Public private import DataFlowImpl import DataFlowImplCommonPublic -import FlowStateString +deprecated import FlowStateString private import codeql.util.Unit /** diff --git a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImpl3.qll b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImpl3.qll index 1a8158437bf..2bbc565daa6 100644 --- a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImpl3.qll +++ b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImpl3.qll @@ -10,7 +10,7 @@ private import DataFlowImplSpecific::Private import DataFlowImplSpecific::Public private import DataFlowImpl import DataFlowImplCommonPublic -import FlowStateString +deprecated import FlowStateString private import codeql.util.Unit /** diff --git a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImpl4.qll b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImpl4.qll index 1a8158437bf..2bbc565daa6 100644 --- a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImpl4.qll +++ b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImpl4.qll @@ -10,7 +10,7 @@ private import DataFlowImplSpecific::Private import DataFlowImplSpecific::Public private import DataFlowImpl import DataFlowImplCommonPublic -import FlowStateString +deprecated import FlowStateString private import codeql.util.Unit /** diff --git a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImpl5.qll b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImpl5.qll index 1a8158437bf..2bbc565daa6 100644 --- a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImpl5.qll +++ b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImpl5.qll @@ -10,7 +10,7 @@ private import DataFlowImplSpecific::Private import DataFlowImplSpecific::Public private import DataFlowImpl import DataFlowImplCommonPublic -import FlowStateString +deprecated import FlowStateString private import codeql.util.Unit /** diff --git a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImpl6.qll b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImpl6.qll index 1a8158437bf..2bbc565daa6 100644 --- a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImpl6.qll +++ b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImpl6.qll @@ -10,7 +10,7 @@ private import DataFlowImplSpecific::Private import DataFlowImplSpecific::Public private import DataFlowImpl import DataFlowImplCommonPublic -import FlowStateString +deprecated import FlowStateString private import codeql.util.Unit /** diff --git a/java/ql/lib/semmle/code/java/security/ImplicitPendingIntents.qll b/java/ql/lib/semmle/code/java/security/ImplicitPendingIntents.qll index f26cef37251..4cac7715b98 100644 --- a/java/ql/lib/semmle/code/java/security/ImplicitPendingIntents.qll +++ b/java/ql/lib/semmle/code/java/security/ImplicitPendingIntents.qll @@ -6,16 +6,44 @@ private import semmle.code.java.dataflow.TaintTracking private import semmle.code.java.frameworks.android.Intent private import semmle.code.java.frameworks.android.PendingIntent +private newtype TPendingIntentState = + TMutablePendingIntent() or + TNoState() + +/** A flow state for an implicit `PendingIntent` flow. */ +class PendingIntentState extends TPendingIntentState { + /** Gets a textual representation of this element. */ + abstract string toString(); +} + +/** A flow state indicating that a mutable `PendingIntent` has been created. */ +class MutablePendingIntent extends PendingIntentState, TMutablePendingIntent { + override string toString() { result = "MutablePendingIntent" } +} + +/** The initial flow state for an implicit `PendingIntent` flow. */ +class NoState extends PendingIntentState, TNoState { + override string toString() { result = "NoState" } +} + /** A source for an implicit `PendingIntent` flow. */ abstract class ImplicitPendingIntentSource extends DataFlow::Node { - /** Holds if this source has the specified `state`. */ - predicate hasState(DataFlow::FlowState state) { state = "" } + /** + * DEPRECATED: Open-ended flow state is not intended to be part of the extension points. + * + * Holds if this source has the specified `state`. + */ + deprecated predicate hasState(DataFlow::FlowState state) { state = "" } } /** A sink that sends an implicit and mutable `PendingIntent` to a third party. */ abstract class ImplicitPendingIntentSink extends DataFlow::Node { - /** Holds if this sink has the specified `state`. */ - predicate hasState(DataFlow::FlowState state) { state = "" } + /** + * DEPRECATED: Open-ended flow state is not intended to be part of the extension points. + * + * Holds if this sink has the specified `state`. + */ + deprecated predicate hasState(DataFlow::FlowState state) { state = "" } } /** @@ -32,11 +60,19 @@ class ImplicitPendingIntentAdditionalTaintStep extends Unit { predicate step(DataFlow::Node node1, DataFlow::Node node2) { none() } /** + * Holds if the step from `node1` to `node2` creates a mutable `PendingIntent`. + */ + predicate mutablePendingIntentCreation(DataFlow::Node node1, DataFlow::Node node2) { none() } + + /** + * DEPRECATED: Open-ended flow state is not intended to be part of the extension points. + * Use `mutablePendingIntentCreation` instead. + * * Holds if the step from `node1` to `node2` should be considered a taint * step for flows related to the use of implicit `PendingIntent`s. This step is only applicable * in `state1` and updates the flow state to `state2`. */ - predicate step( + deprecated predicate step( DataFlow::Node node1, DataFlow::FlowState state1, DataFlow::Node node2, DataFlow::FlowState state2 ) { @@ -66,17 +102,10 @@ private class SendPendingIntent extends ImplicitPendingIntentSink { or sinkNode(this, "pending-intents") } - - override predicate hasState(DataFlow::FlowState state) { state = "MutablePendingIntent" } } private class MutablePendingIntentFlowStep extends ImplicitPendingIntentAdditionalTaintStep { - override predicate step( - DataFlow::Node node1, DataFlow::FlowState state1, DataFlow::Node node2, - DataFlow::FlowState state2 - ) { - state1 = "" and - state2 = "MutablePendingIntent" and + override predicate mutablePendingIntentCreation(DataFlow::Node node1, DataFlow::Node node2) { exists(PendingIntentCreation pic, Argument flagArg | node1.asExpr() = pic.getIntentArg() and node2.asExpr() = pic and diff --git a/java/ql/lib/semmle/code/java/security/ImplicitPendingIntentsQuery.qll b/java/ql/lib/semmle/code/java/security/ImplicitPendingIntentsQuery.qll index 9ef330459bf..402dacb2e9a 100644 --- a/java/ql/lib/semmle/code/java/security/ImplicitPendingIntentsQuery.qll +++ b/java/ql/lib/semmle/code/java/security/ImplicitPendingIntentsQuery.qll @@ -60,14 +60,14 @@ deprecated class ImplicitPendingIntentStartConf extends TaintTracking::Configura * being wrapped in another implicit `Intent` that gets started. */ module ImplicitPendingIntentStartConfig implements DataFlow::StateConfigSig { - class FlowState = DataFlow::FlowState; + class FlowState = PendingIntentState; predicate isSource(DataFlow::Node source, FlowState state) { - source.(ImplicitPendingIntentSource).hasState(state) + source instanceof ImplicitPendingIntentSource and state instanceof NoState } predicate isSink(DataFlow::Node sink, FlowState state) { - sink.(ImplicitPendingIntentSink).hasState(state) + sink instanceof ImplicitPendingIntentSink and state instanceof MutablePendingIntent } predicate isBarrier(DataFlow::Node sanitizer) { sanitizer instanceof ExplicitIntentSanitizer } @@ -79,7 +79,9 @@ module ImplicitPendingIntentStartConfig implements DataFlow::StateConfigSig { predicate isAdditionalFlowStep( DataFlow::Node node1, FlowState state1, DataFlow::Node node2, FlowState state2 ) { - any(ImplicitPendingIntentAdditionalTaintStep c).step(node1, state1, node2, state2) + any(ImplicitPendingIntentAdditionalTaintStep c).mutablePendingIntentCreation(node1, node2) and + state1 instanceof NoState and + state2 instanceof MutablePendingIntent } predicate allowImplicitRead(DataFlow::Node node, DataFlow::ContentSet c) { diff --git a/java/ql/lib/semmle/code/java/security/InsufficientKeySize.qll b/java/ql/lib/semmle/code/java/security/InsufficientKeySize.qll index 949657df654..1f80136fdf1 100644 --- a/java/ql/lib/semmle/code/java/security/InsufficientKeySize.qll +++ b/java/ql/lib/semmle/code/java/security/InsufficientKeySize.qll @@ -3,32 +3,76 @@ private import semmle.code.java.security.Encryption private import semmle.code.java.dataflow.DataFlow private import semmle.code.java.security.internal.EncryptionKeySizes +private import codeql.util.Either + +/** A minimum recommended key size for some algorithm. */ +abstract class MinimumKeySize extends int { + bindingset[this] + MinimumKeySize() { any() } + + /** Gets a textual representation of this element. */ + string toString() { result = super.toString() } +} + +/** + * A class of algorithms for which a key size smaller than the recommended key + * size might be embedded in the algorithm name. + */ +abstract class AlgorithmKind extends string { + bindingset[this] + AlgorithmKind() { any() } + + /** Gets a textual representation of this element. */ + string toString() { result = super.toString() } +} + +/** + * A key size that is greater than the tracked value and equal to the minimum + * recommended key size for some algorithm, or a kind of algorithm for which the + * tracked string indicates a too small key size. + */ +final class KeySizeState = Either::Either; /** A source for an insufficient key size. */ abstract class InsufficientKeySizeSource extends DataFlow::Node { /** Holds if this source has the specified `state`. */ - predicate hasState(DataFlow::FlowState state) { state instanceof DataFlow::FlowStateEmpty } + abstract predicate hasState(KeySizeState state); } /** A sink for an insufficient key size. */ abstract class InsufficientKeySizeSink extends DataFlow::Node { - /** Holds if this sink has the specified `state`. */ - predicate hasState(DataFlow::FlowState state) { state instanceof DataFlow::FlowStateEmpty } + /** Holds if this sink accepts the specified `state`. */ + final predicate hasState(KeySizeState state) { + state.asLeft() <= this.minimumKeySize() or this.algorithmKind(state.asRight()) + } + + /** Gets the minimum recommended key size. */ + abstract int minimumKeySize(); + + /** + * Holds if this sink recommends a keysize that is greater than the value in a + * source with the given algorithm kind. + */ + predicate algorithmKind(AlgorithmKind kind) { none() } +} + +/** A source for an insufficient key size used in some algorithm. */ +private class IntegerLiteralSource extends InsufficientKeySizeSource { + private int value; + + IntegerLiteralSource() { this.asExpr().(IntegerLiteral).getIntValue() = value } + + override predicate hasState(KeySizeState state) { + state.asLeft() = min(MinimumKeySize m | value < m) + } } /** Provides models for asymmetric cryptography. */ private module Asymmetric { /** Provides models for non-elliptic-curve asymmetric cryptography. */ private module NonEllipticCurve { - /** A source for an insufficient key size used in RSA, DSA, and DH algorithms. */ - private class Source extends InsufficientKeySizeSource { - string algoName; - - Source() { this.asExpr().(IntegerLiteral).getIntValue() < getMinKeySize(algoName) } - - override predicate hasState(DataFlow::FlowState state) { - state = getMinKeySize(algoName).toString() - } + private class NonEllipticCurveKeySize extends MinimumKeySize { + NonEllipticCurveKeySize() { this = getMinKeySize(_) } } /** A sink for an insufficient key size used in RSA, DSA, and DH algorithms. */ @@ -46,9 +90,7 @@ private module Asymmetric { exists(Spec spec | this.asExpr() = spec.getKeySizeArg() and algoName = spec.getAlgoName()) } - override predicate hasState(DataFlow::FlowState state) { - state = getMinKeySize(algoName).toString() - } + override int minimumKeySize() { result = getMinKeySize(algoName) } } /** Returns the minimum recommended key size for RSA, DSA, and DH algorithms. */ @@ -88,16 +130,24 @@ private module Asymmetric { /** Provides models for elliptic-curve asymmetric cryptography. */ private module EllipticCurve { + private class EllipticCurveKeySize extends MinimumKeySize { + EllipticCurveKeySize() { this = getMinKeySize() } + } + + private class EllipticCurveKind extends AlgorithmKind { + EllipticCurveKind() { this = "EC" } + } + /** A source for an insufficient key size used in elliptic curve (EC) algorithms. */ private class Source extends InsufficientKeySizeSource { Source() { - this.asExpr().(IntegerLiteral).getIntValue() < getMinKeySize() - or // the below is needed for cases when the key size is embedded in the curve name getKeySize(this.asExpr().(StringLiteral).getValue()) < getMinKeySize() } - override predicate hasState(DataFlow::FlowState state) { state = getMinKeySize().toString() } + override predicate hasState(KeySizeState state) { + state.asRight() instanceof EllipticCurveKind + } } /** A sink for an insufficient key size used in elliptic curve (EC) algorithms. */ @@ -112,7 +162,9 @@ private module Asymmetric { exists(Spec s | this.asExpr() = s.getKeySizeArg()) } - override predicate hasState(DataFlow::FlowState state) { state = getMinKeySize().toString() } + override int minimumKeySize() { result = getMinKeySize() } + + override predicate algorithmKind(AlgorithmKind kind) { kind instanceof EllipticCurveKind } } /** Returns the minimum recommended key size for elliptic curve (EC) algorithms. */ @@ -176,11 +228,8 @@ private module Asymmetric { /** Provides models for symmetric cryptography. */ private module Symmetric { - /** A source for an insufficient key size used in AES algorithms. */ - private class Source extends InsufficientKeySizeSource { - Source() { this.asExpr().(IntegerLiteral).getIntValue() < getMinKeySize() } - - override predicate hasState(DataFlow::FlowState state) { state = getMinKeySize().toString() } + private class SymmetricKeySize extends MinimumKeySize { + SymmetricKeySize() { this = getMinKeySize() } } /** A sink for an insufficient key size used in AES algorithms. */ @@ -193,7 +242,7 @@ private module Symmetric { ) } - override predicate hasState(DataFlow::FlowState state) { state = getMinKeySize().toString() } + override int minimumKeySize() { result = getMinKeySize() } } /** Returns the minimum recommended key size for AES algorithms. */ diff --git a/java/ql/lib/semmle/code/java/security/InsufficientKeySizeQuery.qll b/java/ql/lib/semmle/code/java/security/InsufficientKeySizeQuery.qll index d0d51ffbf08..67678d72a28 100644 --- a/java/ql/lib/semmle/code/java/security/InsufficientKeySizeQuery.qll +++ b/java/ql/lib/semmle/code/java/security/InsufficientKeySizeQuery.qll @@ -12,11 +12,11 @@ deprecated class KeySizeConfiguration extends DataFlow::Configuration { KeySizeConfiguration() { this = "KeySizeConfiguration" } override predicate isSource(DataFlow::Node source, DataFlow::FlowState state) { - source.(InsufficientKeySizeSource).hasState(state) + exists(KeySizeState s | source.(InsufficientKeySizeSource).hasState(s) and state = s.toString()) } override predicate isSink(DataFlow::Node sink, DataFlow::FlowState state) { - sink.(InsufficientKeySizeSink).hasState(state) + exists(KeySizeState s | sink.(InsufficientKeySizeSink).hasState(s) and state = s.toString()) } } @@ -24,24 +24,15 @@ deprecated class KeySizeConfiguration extends DataFlow::Configuration { * A data flow configuration for tracking key sizes used in cryptographic algorithms. */ module KeySizeConfig implements DataFlow::StateConfigSig { - class FlowState = DataFlow::FlowState; + class FlowState = KeySizeState; - predicate isSource(DataFlow::Node source, DataFlow::FlowState state) { + predicate isSource(DataFlow::Node source, KeySizeState state) { source.(InsufficientKeySizeSource).hasState(state) } - predicate isSink(DataFlow::Node sink, DataFlow::FlowState state) { + predicate isSink(DataFlow::Node sink, KeySizeState state) { sink.(InsufficientKeySizeSink).hasState(state) } - - predicate isBarrier(DataFlow::Node node, DataFlow::FlowState state) { none() } - - predicate isAdditionalFlowStep( - DataFlow::Node node1, DataFlow::FlowState state1, DataFlow::Node node2, - DataFlow::FlowState state2 - ) { - none() - } } /** Tracks key sizes used in cryptographic algorithms. */ diff --git a/java/ql/lib/semmle/code/java/security/TemplateInjection.qll b/java/ql/lib/semmle/code/java/security/TemplateInjection.qll index b3e9bb86aaa..bb212d39c7d 100644 --- a/java/ql/lib/semmle/code/java/security/TemplateInjection.qll +++ b/java/ql/lib/semmle/code/java/security/TemplateInjection.qll @@ -9,16 +9,28 @@ private import semmle.code.java.dataflow.TaintTracking * A source for server-side template injection (SST) vulnerabilities. */ abstract class TemplateInjectionSource extends DataFlow::Node { - /** Holds if this source has the specified `state`. */ - predicate hasState(DataFlow::FlowState state) { state instanceof DataFlow::FlowStateEmpty } + /** + * DEPRECATED: Open-ended flow state is not intended to be part of the extension points. + * + * Holds if this source has the specified `state`. + */ + deprecated predicate hasState(DataFlow::FlowState state) { + state instanceof DataFlow::FlowStateEmpty + } } /** * A sink for server-side template injection (SST) vulnerabilities. */ abstract class TemplateInjectionSink extends DataFlow::Node { - /** Holds if this sink has the specified `state`. */ - predicate hasState(DataFlow::FlowState state) { state instanceof DataFlow::FlowStateEmpty } + /** + * DEPRECATED: Open-ended flow state is not intended to be part of the extension points. + * + * Holds if this sink has the specified `state`. + */ + deprecated predicate hasState(DataFlow::FlowState state) { + state instanceof DataFlow::FlowStateEmpty + } } /** @@ -35,11 +47,13 @@ class TemplateInjectionAdditionalTaintStep extends Unit { predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) { none() } /** + * DEPRECATED: Open-ended flow state is not intended to be part of the extension points. + * * Holds if the step from `node1` to `node2` should be considered a taint * step for flows related toserver-side template injection (SST) vulnerabilities. * This step is only applicable in `state1` and updates the flow state to `state2`. */ - predicate isAdditionalTaintStep( + deprecated predicate isAdditionalTaintStep( DataFlow::Node node1, DataFlow::FlowState state1, DataFlow::Node node2, DataFlow::FlowState state2 ) { @@ -53,13 +67,19 @@ class TemplateInjectionAdditionalTaintStep extends Unit { abstract class TemplateInjectionSanitizer extends DataFlow::Node { } /** + * DEPRECATED: Open-ended flow state is not intended to be part of the extension points. + * * A sanitizer for server-side template injection (SST) vulnerabilities. * This sanitizer is only applicable when `TemplateInjectionSanitizerWithState::hasState` * holds for the flow state. */ -abstract class TemplateInjectionSanitizerWithState extends DataFlow::Node { - /** Holds if this sanitizer has the specified `state`. */ - abstract predicate hasState(DataFlow::FlowState state); +abstract deprecated class TemplateInjectionSanitizerWithState extends DataFlow::Node { + /** + * DEPRECATED: Open-ended flow state is not intended to be part of the extension points. + * + * Holds if this sanitizer has the specified `state`. + */ + abstract deprecated predicate hasState(DataFlow::FlowState state); } private class DefaultTemplateInjectionSource extends TemplateInjectionSource instanceof ThreatModelFlowSource diff --git a/java/ql/lib/semmle/code/java/security/TemplateInjectionQuery.qll b/java/ql/lib/semmle/code/java/security/TemplateInjectionQuery.qll index 783c368482c..07150b554aa 100644 --- a/java/ql/lib/semmle/code/java/security/TemplateInjectionQuery.qll +++ b/java/ql/lib/semmle/code/java/security/TemplateInjectionQuery.qll @@ -42,33 +42,17 @@ deprecated class TemplateInjectionFlowConfig extends TaintTracking::Configuratio } /** A taint tracking configuration to reason about server-side template injection (SST) vulnerabilities */ -module TemplateInjectionFlowConfig implements DataFlow::StateConfigSig { - class FlowState = DataFlow::FlowState; +module TemplateInjectionFlowConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source instanceof TemplateInjectionSource } - predicate isSource(DataFlow::Node source, FlowState state) { - source.(TemplateInjectionSource).hasState(state) - } - - predicate isSink(DataFlow::Node sink, FlowState state) { - sink.(TemplateInjectionSink).hasState(state) - } + predicate isSink(DataFlow::Node sink) { sink instanceof TemplateInjectionSink } predicate isBarrier(DataFlow::Node sanitizer) { sanitizer instanceof TemplateInjectionSanitizer } - predicate isBarrier(DataFlow::Node sanitizer, FlowState state) { - sanitizer.(TemplateInjectionSanitizerWithState).hasState(state) - } - predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { any(TemplateInjectionAdditionalTaintStep a).isAdditionalTaintStep(node1, node2) } - - predicate isAdditionalFlowStep( - DataFlow::Node node1, FlowState state1, DataFlow::Node node2, FlowState state2 - ) { - any(TemplateInjectionAdditionalTaintStep a).isAdditionalTaintStep(node1, state1, node2, state2) - } } /** Tracks server-side template injection (SST) vulnerabilities */ -module TemplateInjectionFlow = TaintTracking::GlobalWithState; +module TemplateInjectionFlow = TaintTracking::Global; diff --git a/java/ql/src/change-notes/2023-12-14-flowstatestring-deprecated.md b/java/ql/src/change-notes/2023-12-14-flowstatestring-deprecated.md new file mode 100644 index 00000000000..580f88c7807 --- /dev/null +++ b/java/ql/src/change-notes/2023-12-14-flowstatestring-deprecated.md @@ -0,0 +1,4 @@ +--- +category: deprecated +--- +* The three queries `java/insufficient-key-size`, `java/server-side-template-injection`, and `java/android/implicit-pendingintents` had accidentally general extension points allowing arbitrary string-based flow state. This has been fixed and the old extension points have been deprecated where possible, and otherwise updated. diff --git a/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImpl1.qll b/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImpl1.qll index 1a8158437bf..2bbc565daa6 100644 --- a/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImpl1.qll +++ b/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImpl1.qll @@ -10,7 +10,7 @@ private import DataFlowImplSpecific::Private import DataFlowImplSpecific::Public private import DataFlowImpl import DataFlowImplCommonPublic -import FlowStateString +deprecated import FlowStateString private import codeql.util.Unit /** diff --git a/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImpl2.qll b/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImpl2.qll index 1a8158437bf..2bbc565daa6 100644 --- a/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImpl2.qll +++ b/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImpl2.qll @@ -10,7 +10,7 @@ private import DataFlowImplSpecific::Private import DataFlowImplSpecific::Public private import DataFlowImpl import DataFlowImplCommonPublic -import FlowStateString +deprecated import FlowStateString private import codeql.util.Unit /** diff --git a/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImpl3.qll b/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImpl3.qll index 1a8158437bf..2bbc565daa6 100644 --- a/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImpl3.qll +++ b/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImpl3.qll @@ -10,7 +10,7 @@ private import DataFlowImplSpecific::Private import DataFlowImplSpecific::Public private import DataFlowImpl import DataFlowImplCommonPublic -import FlowStateString +deprecated import FlowStateString private import codeql.util.Unit /** diff --git a/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImpl4.qll b/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImpl4.qll index 1a8158437bf..2bbc565daa6 100644 --- a/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImpl4.qll +++ b/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImpl4.qll @@ -10,7 +10,7 @@ private import DataFlowImplSpecific::Private import DataFlowImplSpecific::Public private import DataFlowImpl import DataFlowImplCommonPublic -import FlowStateString +deprecated import FlowStateString private import codeql.util.Unit /** diff --git a/python/ql/lib/semmle/python/security/dataflow/PathInjectionQuery.qll b/python/ql/lib/semmle/python/security/dataflow/PathInjectionQuery.qll index c3ee07d805d..2cd6ba2a6f4 100644 --- a/python/ql/lib/semmle/python/security/dataflow/PathInjectionQuery.qll +++ b/python/ql/lib/semmle/python/security/dataflow/PathInjectionQuery.qll @@ -63,13 +63,18 @@ deprecated class Configuration extends TaintTracking::Configuration { } } +abstract private class NormalizationState extends string { + bindingset[this] + NormalizationState() { any() } +} + /** A state signifying that the file path has not been normalized. */ -class NotNormalized extends DataFlow::FlowState { +class NotNormalized extends NormalizationState { NotNormalized() { this = "NotNormalized" } } /** A state signifying that the file path has been normalized, but not checked. */ -class NormalizedUnchecked extends DataFlow::FlowState { +class NormalizedUnchecked extends NormalizationState { NormalizedUnchecked() { this = "NormalizedUnchecked" } } @@ -85,7 +90,7 @@ class NormalizedUnchecked extends DataFlow::FlowState { * Such checks are ineffective in the `NotNormalized` state. */ module PathInjectionConfig implements DataFlow::StateConfigSig { - class FlowState = DataFlow::FlowState; + class FlowState = NormalizationState; predicate isSource(DataFlow::Node source, FlowState state) { source instanceof Source and state instanceof NotNormalized diff --git a/python/ql/src/experimental/Security/CWE-176/UnicodeBypassValidationQuery.qll b/python/ql/src/experimental/Security/CWE-176/UnicodeBypassValidationQuery.qll index a5d9d53b084..f2c3b01ac30 100644 --- a/python/ql/src/experimental/Security/CWE-176/UnicodeBypassValidationQuery.qll +++ b/python/ql/src/experimental/Security/CWE-176/UnicodeBypassValidationQuery.qll @@ -12,13 +12,18 @@ import semmle.python.dataflow.new.internal.TaintTrackingPrivate import semmle.python.dataflow.new.RemoteFlowSources import UnicodeBypassValidationCustomizations::UnicodeBypassValidation +abstract private class ValidationState extends string { + bindingset[this] + ValidationState() { any() } +} + /** A state signifying that a logical validation has not been performed. */ -class PreValidation extends DataFlow::FlowState { +class PreValidation extends ValidationState { PreValidation() { this = "PreValidation" } } /** A state signifying that a logical validation has been performed. */ -class PostValidation extends DataFlow::FlowState { +class PostValidation extends ValidationState { PostValidation() { this = "PostValidation" } } @@ -29,7 +34,7 @@ class PostValidation extends DataFlow::FlowState { * to track the requirement that a logical validation has been performed before the Unicode Transformation. */ private module UnicodeBypassValidationConfig implements DataFlow::StateConfigSig { - class FlowState = DataFlow::FlowState; + class FlowState = ValidationState; predicate isSource(DataFlow::Node source, FlowState state) { source instanceof RemoteFlowSource and state instanceof PreValidation diff --git a/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowImpl1.qll b/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowImpl1.qll index 1a8158437bf..2bbc565daa6 100644 --- a/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowImpl1.qll +++ b/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowImpl1.qll @@ -10,7 +10,7 @@ private import DataFlowImplSpecific::Private import DataFlowImplSpecific::Public private import DataFlowImpl import DataFlowImplCommonPublic -import FlowStateString +deprecated import FlowStateString private import codeql.util.Unit /** diff --git a/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowImpl2.qll b/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowImpl2.qll index 1a8158437bf..2bbc565daa6 100644 --- a/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowImpl2.qll +++ b/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowImpl2.qll @@ -10,7 +10,7 @@ private import DataFlowImplSpecific::Private import DataFlowImplSpecific::Public private import DataFlowImpl import DataFlowImplCommonPublic -import FlowStateString +deprecated import FlowStateString private import codeql.util.Unit /** diff --git a/shared/dataflow/codeql/dataflow/internal/DataFlowImplCommon.qll b/shared/dataflow/codeql/dataflow/internal/DataFlowImplCommon.qll index ea1378ce05d..04937454504 100644 --- a/shared/dataflow/codeql/dataflow/internal/DataFlowImplCommon.qll +++ b/shared/dataflow/codeql/dataflow/internal/DataFlowImplCommon.qll @@ -7,16 +7,22 @@ module MakeImplCommon { import Cached module DataFlowImplCommonPublic { - /** Provides `FlowState = string`. */ - module FlowStateString { + /** + * DEPRECATED: Generally, a custom `FlowState` type should be used instead, + * but `string` can of course still be used without referring to this + * module. + * + * Provides `FlowState = string`. + */ + deprecated module FlowStateString { /** A state value to track during data flow. */ - class FlowState = string; + deprecated class FlowState = string; /** * The default state, which is used when the state is unspecified for a source * or a sink. */ - class FlowStateEmpty extends FlowState { + deprecated class FlowStateEmpty extends FlowState { FlowStateEmpty() { this = "" } } } diff --git a/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowImpl1.qll b/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowImpl1.qll index 1a8158437bf..2bbc565daa6 100644 --- a/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowImpl1.qll +++ b/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowImpl1.qll @@ -10,7 +10,7 @@ private import DataFlowImplSpecific::Private import DataFlowImplSpecific::Public private import DataFlowImpl import DataFlowImplCommonPublic -import FlowStateString +deprecated import FlowStateString private import codeql.util.Unit /**