diff --git a/java/ql/lib/semmle/code/java/frameworks/JaxWS.qll b/java/ql/lib/semmle/code/java/frameworks/JaxWS.qll index a0f891fd36ea..e9f1cea4b330 100644 --- a/java/ql/lib/semmle/code/java/frameworks/JaxWS.qll +++ b/java/ql/lib/semmle/code/java/frameworks/JaxWS.qll @@ -417,7 +417,7 @@ class JaxRSConsumesAnnotation extends JaxRSAnnotation { } /** A default sink representing methods susceptible to XSS attacks. */ -private class JaxRSXssSink extends XssSink { +private class JaxRSXssSink extends AbstractXssSink { JaxRSXssSink() { exists(JaxRsResourceMethod resourceMethod, ReturnStmt rs | resourceMethod = any(JaxRsResourceClass resourceClass).getAResourceMethod() and diff --git a/java/ql/lib/semmle/code/java/frameworks/spring/SpringHttp.qll b/java/ql/lib/semmle/code/java/frameworks/spring/SpringHttp.qll index e12e2b2643a0..25f7fdc3068d 100644 --- a/java/ql/lib/semmle/code/java/frameworks/spring/SpringHttp.qll +++ b/java/ql/lib/semmle/code/java/frameworks/spring/SpringHttp.qll @@ -46,7 +46,7 @@ private predicate specifiesContentType(SpringRequestMappingMethod method) { exists(method.getAProducesExpr()) } -private class SpringXssSink extends XSS::XssSink { +private class SpringXssSink extends XSS::AbstractXssSink { SpringXssSink() { exists(SpringRequestMappingMethod requestMappingMethod, ReturnStmt rs | requestMappingMethod = rs.getEnclosingCallable() and diff --git a/java/ql/lib/semmle/code/java/regex/RegexDiffInformed.qll b/java/ql/lib/semmle/code/java/regex/RegexDiffInformed.qll new file mode 100644 index 000000000000..004a70bf7a25 --- /dev/null +++ b/java/ql/lib/semmle/code/java/regex/RegexDiffInformed.qll @@ -0,0 +1,28 @@ +private import java +private import semmle.code.java.dataflow.DataFlow +private import codeql.util.Unit + +/** + * An extension point to allow a query to detect only the regular expressions + * it needs in diff-informed incremental mode. The data-flow analysis that's + * modified by this class has its sources as (certain) string literals and its + * sinks as regular-expression matches. + */ +class RegexDiffInformedConfig instanceof Unit { + /** + * Holds if discovery of regular expressions should be diff-informed, which + * is possible when there cannot be any elements selected by the query in the + * diff range except the regular expressions and (locations derived from) the + * places where they are matched against. + */ + abstract predicate observeDiffInformedIncrementalMode(); + + /** + * Gets a location of a regex match that will be part of the query results. + * If the query does not select the match locations, this predicate can be + * `none()` for performance. + */ + abstract Location getASelectedSinkLocation(DataFlow::Node sink); + + string toString() { result = "RegexDiffInformedConfig" } +} diff --git a/java/ql/lib/semmle/code/java/regex/RegexFlowConfigs.qll b/java/ql/lib/semmle/code/java/regex/RegexFlowConfigs.qll index 763b96f5a02d..e04f849c16a5 100644 --- a/java/ql/lib/semmle/code/java/regex/RegexFlowConfigs.qll +++ b/java/ql/lib/semmle/code/java/regex/RegexFlowConfigs.qll @@ -6,6 +6,7 @@ import java import semmle.code.java.dataflow.ExternalFlow private import semmle.code.java.dataflow.DataFlow private import semmle.code.java.security.SecurityTests +private import RegexDiffInformed private class ExploitableStringLiteral extends StringLiteral { ExploitableStringLiteral() { this.getValue().matches(["%+%", "%*%", "%{%}%"]) } @@ -157,6 +158,14 @@ private module RegexFlowConfig implements DataFlow::ConfigSig { } int fieldFlowBranchLimit() { result = 1 } + + predicate observeDiffInformedIncrementalMode() { + exists(RegexDiffInformedConfig c | c.observeDiffInformedIncrementalMode()) + } + + Location getASelectedSinkLocation(DataFlow::Node sink) { + exists(RegexDiffInformedConfig c | result = c.getASelectedSinkLocation(sink)) + } } private module RegexFlow = DataFlow::Global; diff --git a/java/ql/lib/semmle/code/java/security/CleartextStorageCookieQuery.qll b/java/ql/lib/semmle/code/java/security/CleartextStorageCookieQuery.qll index 1f262ad57d61..1c99821386da 100644 --- a/java/ql/lib/semmle/code/java/security/CleartextStorageCookieQuery.qll +++ b/java/ql/lib/semmle/code/java/security/CleartextStorageCookieQuery.qll @@ -7,7 +7,17 @@ private import semmle.code.java.dataflow.FlowSinks private import semmle.code.java.dataflow.FlowSources private class CookieCleartextStorageSink extends CleartextStorageSink { - CookieCleartextStorageSink() { this.asExpr() = cookieInput(_) } + Cookie cookie; + + CookieCleartextStorageSink() { this.asExpr() = cookieInput(cookie) } + + override Location getASelectedLocation() { + result = this.getLocation() + or + result = cookie.getLocation() + or + result = cookie.getAStore().getLocation() + } } /** The instantiation of a cookie, which can act as storage. */ diff --git a/java/ql/lib/semmle/code/java/security/CleartextStorageQuery.qll b/java/ql/lib/semmle/code/java/security/CleartextStorageQuery.qll index a607fd8c8d2b..21d82bef657e 100644 --- a/java/ql/lib/semmle/code/java/security/CleartextStorageQuery.qll +++ b/java/ql/lib/semmle/code/java/security/CleartextStorageQuery.qll @@ -5,7 +5,14 @@ private import semmle.code.java.dataflow.TaintTracking private import semmle.code.java.security.SensitiveActions /** A sink representing persistent storage that saves data in clear text. */ -abstract class CleartextStorageSink extends DataFlow::Node { } +abstract class CleartextStorageSink extends DataFlow::Node { + /** + * Gets a location that will be selected in the diff-informed query where + * this sink is found. If this has no results for any sink, that's taken to + * mean the query is not diff-informed. + */ + Location getASelectedLocation() { none() } +} /** A sanitizer for flows tracking sensitive data being stored in persistent storage. */ abstract class CleartextStorageSanitizer extends DataFlow::Node { } @@ -46,6 +53,17 @@ private module SensitiveSourceFlowConfig implements DataFlow::ConfigSig { predicate isAdditionalFlowStep(DataFlow::Node n1, DataFlow::Node n2) { any(CleartextStorageAdditionalTaintStep c).step(n1, n2) } + + predicate observeDiffInformedIncrementalMode() { + // This configuration is used by several queries. A query can opt in to + // diff-informed mode by implementing `getASelectedLocation` on its sinks, + // indicating that it has considered which sinks are selected. + exists(CleartextStorageSink sink | exists(sink.getASelectedLocation())) + } + + Location getASelectedSinkLocation(DataFlow::Node sink) { + result = sink.(CleartextStorageSink).getASelectedLocation() + } } private module SensitiveSourceFlow = TaintTracking::Global; diff --git a/java/ql/lib/semmle/code/java/security/InformationLeak.qll b/java/ql/lib/semmle/code/java/security/InformationLeak.qll index 8fe7d2151650..e95d00edc8bf 100644 --- a/java/ql/lib/semmle/code/java/security/InformationLeak.qll +++ b/java/ql/lib/semmle/code/java/security/InformationLeak.qll @@ -5,13 +5,44 @@ import semmle.code.java.dataflow.DataFlow private import semmle.code.java.dataflow.ExternalFlow import semmle.code.java.security.XSS -/** A sink that represent a method that outputs data to an HTTP response. */ -abstract class InformationLeakSink extends DataFlow::Node { } +/** + * A sink that represent a method that outputs data to an HTTP response. Extend + * this class to add more sinks that should be considered information leak + * points by every query. To find the full set of information-leak sinks, use + * `InformationLeakSink` instead. + */ +abstract class AbstractInformationLeakSink extends DataFlow::Node { } + +/** + * A sink that represent a method that outputs data to an HTTP response. To add + * more sinks, extend `AbstractInformationLeakSink` rather than this class. + */ +final class InformationLeakSink extends DataFlow::Node instanceof InformationLeakDiffInformed::InformationLeakSink +{ } /** A default sink representing methods outputing data to an HTTP response. */ -private class DefaultInformationLeakSink extends InformationLeakSink { - DefaultInformationLeakSink() { - sinkNode(this, "information-leak") or - this instanceof XssSink +private class DefaultInformationLeakSink extends AbstractInformationLeakSink { + DefaultInformationLeakSink() { sinkNode(this, "information-leak") } +} + +/** + * A module for finding information-leak sinks faster in a diff-informed query. + * The `hasSourceInDiffRange` parameter should hold if the overall data-flow + * configuration of the query has any sources in the diff range. + */ +module InformationLeakDiffInformed { + final private class Node = DataFlow::Node; + + /** + * A diff-informed replacement for the top-level `InformationLeakSink`, + * omitting for efficiency some sinks that would never be reported by a + * diff-informed query. + */ + final class InformationLeakSink extends Node { + InformationLeakSink() { + this instanceof AbstractInformationLeakSink + or + this instanceof XssDiffInformed::XssSink + } } } diff --git a/java/ql/lib/semmle/code/java/security/SensitiveDataExposureThroughErrorMessageQuery.qll b/java/ql/lib/semmle/code/java/security/SensitiveDataExposureThroughErrorMessageQuery.qll index 8644ef415b3e..3c3bdd7dfc72 100644 --- a/java/ql/lib/semmle/code/java/security/SensitiveDataExposureThroughErrorMessageQuery.qll +++ b/java/ql/lib/semmle/code/java/security/SensitiveDataExposureThroughErrorMessageQuery.qll @@ -20,7 +20,12 @@ private class GetMessageFlowSource extends ApiSourceNode { private module GetMessageFlowSourceToHttpResponseSinkFlowConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node src) { src instanceof GetMessageFlowSource } - predicate isSink(DataFlow::Node sink) { sink instanceof InformationLeakSink } + predicate isSink(DataFlow::Node sink) { + sink instanceof + InformationLeakDiffInformed::InformationLeakSink + } + + predicate observeDiffInformedIncrementalMode() { any() } } private module GetMessageFlowSourceToHttpResponseSinkFlow = diff --git a/java/ql/lib/semmle/code/java/security/StackTraceExposureQuery.qll b/java/ql/lib/semmle/code/java/security/StackTraceExposureQuery.qll index 0eb069a06c20..25cb4b141522 100644 --- a/java/ql/lib/semmle/code/java/security/StackTraceExposureQuery.qll +++ b/java/ql/lib/semmle/code/java/security/StackTraceExposureQuery.qll @@ -25,6 +25,14 @@ private module ServletWriterSourceToPrintStackTraceMethodFlowConfig implements D sink.asExpr() = ma.getAnArgument() and ma.getMethod() instanceof PrintStackTraceMethod ) } + + predicate observeDiffInformedIncrementalMode() { any() } + + Location getASelectedSinkLocation(DataFlow::Node sink) { + exists(MethodCall ma | result = ma.getLocation() | + sink.asExpr() = ma.getAnArgument() and ma.getMethod() instanceof PrintStackTraceMethod + ) + } } private module ServletWriterSourceToPrintStackTraceMethodFlow = @@ -69,7 +77,16 @@ private predicate stackTraceExpr(Expr exception, MethodCall stackTraceString) { private module StackTraceStringToHttpResponseSinkFlowConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node src) { stackTraceExpr(_, src.asExpr()) } - predicate isSink(DataFlow::Node sink) { sink instanceof InformationLeakSink } + predicate isSink(DataFlow::Node sink) { + sink instanceof + InformationLeakDiffInformed::InformationLeakSink + } + + predicate observeDiffInformedIncrementalMode() { any() } + + Location getASelectedSourceLocation(DataFlow::Node source) { + exists(Expr e | stackTraceExpr(e, source.asExpr()) and result = e.getLocation()) + } } private module StackTraceStringToHttpResponseSinkFlow = diff --git a/java/ql/lib/semmle/code/java/security/StaticInitializationVectorQuery.qll b/java/ql/lib/semmle/code/java/security/StaticInitializationVectorQuery.qll index 282133ec5c67..9e74c1f04ce2 100644 --- a/java/ql/lib/semmle/code/java/security/StaticInitializationVectorQuery.qll +++ b/java/ql/lib/semmle/code/java/security/StaticInitializationVectorQuery.qll @@ -81,7 +81,7 @@ private class ArrayUpdate extends Expr { } private predicate arrayUpdateSrc(DataFlow::Node source) { - source.asExpr() instanceof StaticByteArrayCreation + StaticInitializationVectorFlow::flow(source, _) } private predicate arrayUpdateSink(DataFlow::Node sink) { @@ -92,7 +92,7 @@ private module ArrayUpdateFlowFwd = DataFlow::SimpleGlobal; private module ArrayUpdateFlow = ArrayUpdateFlowFwd::Graph; -private predicate arrayReachesUpdate(StaticByteArrayCreation array) { +predicate arrayReachesUpdate(StaticByteArrayCreation array) { exists(ArrayUpdateFlow::PathNode src | src.isSource() and src.getNode().asExpr() = array) } @@ -102,7 +102,6 @@ private predicate arrayReachesUpdate(StaticByteArrayCreation array) { private class StaticInitializationVectorSource extends DataFlow::Node { StaticInitializationVectorSource() { exists(StaticByteArrayCreation array | array = this.asExpr() | - not arrayReachesUpdate(array) and // Reduce FPs from utility methods that return an empty array in an exceptional case not exists(ReturnStmt ret | array.getADimension().(CompileTimeConstantExpr).getIntValue() = 0 and diff --git a/java/ql/lib/semmle/code/java/security/UnsafeDeserializationQuery.qll b/java/ql/lib/semmle/code/java/security/UnsafeDeserializationQuery.qll index b16770c222b8..7c9dfd058045 100644 --- a/java/ql/lib/semmle/code/java/security/UnsafeDeserializationQuery.qll +++ b/java/ql/lib/semmle/code/java/security/UnsafeDeserializationQuery.qll @@ -379,8 +379,12 @@ predicate looksLikeResolveClassStep(DataFlow::Node fromNode, DataFlow::Node toNo /** A sink representing an argument of a deserialization method */ private class UnsafeTypeSink extends DataFlow::Node { + MethodCall ma; + + MethodCall getMethodCall() { result = ma } + UnsafeTypeSink() { - exists(MethodCall ma, int i, Expr arg | i > 0 and ma.getArgument(i) = arg | + exists(int i, Expr arg | i > 0 and ma.getArgument(i) = arg | ( ma.getMethod() instanceof ObjectMapperReadMethod or @@ -425,6 +429,25 @@ module UnsafeTypeConfig implements DataFlow::ConfigSig { predicate isAdditionalFlowStep(DataFlow::Node fromNode, DataFlow::Node toNode) { isUnsafeTypeAdditionalTaintStep(fromNode, toNode) } + + predicate observeDiffInformedIncrementalMode() { + // Since this configuration is for finding sinks to be used in a main + // data-flow configuration, this configuration should only restrict the + // sinks to be found if there are no main-configuration sources in the diff + // range. That's because if there is such a source, we need to report query + // results for it even with sinks outside the diff range. + not UnsafeDeserializationFlow::hasSourceInDiffRange() + } + + // The query does not select the sources of this configuration + Location getASelectedSourceLocation(DataFlow::Node source) { none() } + + Location getASelectedSinkLocation(DataFlow::Node sink) { + // Match by the surrounding method call since the sink of the overall + // query will be contained in that (see the body of + // `unsafeDeserialization/2`). + result = sink.(UnsafeTypeSink).getMethodCall().getLocation() + } } /** diff --git a/java/ql/lib/semmle/code/java/security/XSS.qll b/java/ql/lib/semmle/code/java/security/XSS.qll index cc584033e4fc..da48c056a07c 100644 --- a/java/ql/lib/semmle/code/java/security/XSS.qll +++ b/java/ql/lib/semmle/code/java/security/XSS.qll @@ -13,17 +13,36 @@ private import semmle.code.java.dataflow.ExternalFlow private import semmle.code.java.dataflow.FlowSources private import semmle.code.java.dataflow.FlowSinks -/** A sink that represent a method that outputs data without applying contextual output encoding. */ -abstract class XssSink extends ApiSinkNode { } - /** A sanitizer that neutralizes dangerous characters that can be used to perform a XSS attack. */ abstract class XssSanitizer extends DataFlow::Node { } +/** + * A sink that represent a method that outputs data without applying contextual + * output encoding. Extend this class to add more sinks that should be + * considered XSS sinks by every query. To find the full set of XSS sinks, use + * `XssSink` instead. + */ +abstract class AbstractXssSink extends DataFlow::Node { } + +/** A default sink representing methods susceptible to XSS attacks. */ +private class DefaultXssSink extends AbstractXssSink { + DefaultXssSink() { sinkNode(this, ["html-injection", "js-injection"]) } +} + +/** + * A sink that represent a method that outputs data without applying contextual + * output encoding. To add more sinks, extend `AbstractXssSink` rather than + * this class. To find XSS sinks efficiently for a diff-informed query, use the + * `XssDiffInformed` module instead. + */ +final class XssSink extends ApiSinkNode instanceof XssDiffInformed::XssSink { +} + /** * A sink that represent a method that outputs data without applying contextual output encoding, * and which should truncate flow paths such that downstream sinks are not flagged as well. */ -abstract class XssSinkBarrier extends XssSink { } +abstract class XssSinkBarrier extends AbstractXssSink { } /** * A unit class for adding additional taint steps. @@ -39,19 +58,6 @@ class XssAdditionalTaintStep extends Unit { abstract predicate step(DataFlow::Node node1, DataFlow::Node node2); } -/** A default sink representing methods susceptible to XSS attacks. */ -private class DefaultXssSink extends XssSink { - DefaultXssSink() { - sinkNode(this, ["html-injection", "js-injection"]) - or - exists(MethodCall ma | - ma.getMethod() instanceof WritingMethod and - XssVulnerableWriterSourceToWritingMethodFlow::flowToExpr(ma.getQualifier()) and - this.asExpr() = ma.getArgument(_) - ) - } -} - /** A default sanitizer that considers numeric and boolean typed data safe for writing to output. */ private class DefaultXssSanitizer extends XssSanitizer { DefaultXssSanitizer() { @@ -62,20 +68,6 @@ private class DefaultXssSanitizer extends XssSanitizer { } } -/** A configuration that tracks data from a servlet writer to an output method. */ -private module XssVulnerableWriterSourceToWritingMethodFlowConfig implements DataFlow::ConfigSig { - predicate isSource(DataFlow::Node src) { src instanceof XssVulnerableWriterSourceNode } - - predicate isSink(DataFlow::Node sink) { - exists(MethodCall ma | - sink.asExpr() = ma.getQualifier() and ma.getMethod() instanceof WritingMethod - ) - } -} - -private module XssVulnerableWriterSourceToWritingMethodFlow = - TaintTracking::Global; - /** A method that can be used to output data to an output stream or writer. */ private class WritingMethod extends Method { WritingMethod() { @@ -113,6 +105,77 @@ class XssVulnerableWriterSourceNode extends ApiSourceNode { XssVulnerableWriterSourceNode() { this.asExpr() instanceof XssVulnerableWriterSource } } +/** A nullary predicate. */ +signature predicate xssNullaryPredicate(); + +/** Holds always. Use this predicate as parameter to `XssDiffInformed` to disable diff-informed mode. */ +predicate xssNotDiffInformed() { any() } + +/** + * A module for finding XSS sinks faster in a diff-informed query. The + * `hasSourceInDiffRange` parameter should hold if the overall data-flow + * configuration of the query has any sources in the diff range. + */ +module XssDiffInformed { + final private class Node = DataFlow::Node; + + /** + * A diff-informed replacement for the top-level `XssSink`, omitting for + * efficiency some sinks that would never be reported by a diff-informed + * query. + */ + final class XssSink extends Node { + XssSink() { + this instanceof AbstractXssSink + or + isResponseWriterSink(this) + } + } + + predicate isResponseWriterSink(DataFlow::Node sink) { + exists(MethodCall ma | + // This code mirrors `getASelectedSinkLocation`. + ma.getMethod() instanceof WritingMethod and + XssVulnerableWriterSourceToWritingMethodFlow::flowToExpr(ma.getQualifier()) and + sink.asExpr() = ma.getArgument(_) + ) + } + + /** A configuration that tracks data from a servlet writer to an output method. */ + private module XssVulnerableWriterSourceToWritingMethodFlowConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node src) { src instanceof XssVulnerableWriterSourceNode } + + predicate isSink(DataFlow::Node sink) { + exists(MethodCall ma | + sink.asExpr() = ma.getQualifier() and ma.getMethod() instanceof WritingMethod + ) + } + + predicate observeDiffInformedIncrementalMode() { + // Since this configuration is for finding sinks to be used in a main + // data-flow configuration, this configuration should only restrict the + // sinks to be found if there are no main-configuration sources in the + // diff range. That's because if there is such a source, we need to + // report query results for it even with sinks outside the diff range. + not hasSourceInDiffRange() + } + + // The sources are not exposed outside this file module, so we know the + // query will not select them. + Location getASelectedSourceLocation(DataFlow::Node source) { none() } + + Location getASelectedSinkLocation(DataFlow::Node sink) { + // This code mirrors `isResponseWriterSink`. + exists(MethodCall ma | result = ma.getAnArgument().getLocation() | + sink.asExpr() = ma.getQualifier() and ma.getMethod() instanceof WritingMethod + ) + } + } + + private module XssVulnerableWriterSourceToWritingMethodFlow = + TaintTracking::Global; +} + /** * Holds if `s` is an HTTP Content-Type vulnerable to XSS. */ diff --git a/java/ql/lib/semmle/code/java/security/XssLocalQuery.qll b/java/ql/lib/semmle/code/java/security/XssLocalQuery.qll index 5e1098865aa6..d13583d942d1 100644 --- a/java/ql/lib/semmle/code/java/security/XssLocalQuery.qll +++ b/java/ql/lib/semmle/code/java/security/XssLocalQuery.qll @@ -11,7 +11,9 @@ private import semmle.code.java.security.XSS deprecated module XssLocalConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node source) { source instanceof LocalUserInput } - predicate isSink(DataFlow::Node sink) { sink instanceof XssSink } + predicate isSink(DataFlow::Node sink) { + sink instanceof XssDiffInformed::XssSink + } predicate isBarrier(DataFlow::Node node) { node instanceof XssSanitizer } diff --git a/java/ql/lib/semmle/code/java/security/XssQuery.qll b/java/ql/lib/semmle/code/java/security/XssQuery.qll index c0d7035a4f9a..2576c043f48f 100644 --- a/java/ql/lib/semmle/code/java/security/XssQuery.qll +++ b/java/ql/lib/semmle/code/java/security/XssQuery.qll @@ -11,7 +11,9 @@ import semmle.code.java.security.XSS module XssConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node source) { source instanceof ActiveThreatModelSource } - predicate isSink(DataFlow::Node sink) { sink instanceof XssSink } + predicate isSink(DataFlow::Node sink) { + sink instanceof XssDiffInformed::XssSink + } predicate isBarrier(DataFlow::Node node) { node instanceof XssSanitizer } diff --git a/java/ql/lib/semmle/code/java/security/regexp/PolynomialReDoSQuery.qll b/java/ql/lib/semmle/code/java/security/regexp/PolynomialReDoSQuery.qll index 767ebc97437b..e8eddaebf54c 100644 --- a/java/ql/lib/semmle/code/java/security/regexp/PolynomialReDoSQuery.qll +++ b/java/ql/lib/semmle/code/java/security/regexp/PolynomialReDoSQuery.qll @@ -4,6 +4,7 @@ private import semmle.code.java.regex.RegexTreeView::RegexTreeView as TreeView import codeql.regex.nfa.SuperlinearBackTracking::Make as SuperlinearBackTracking import semmle.code.java.dataflow.DataFlow import semmle.code.java.regex.RegexFlowConfigs +import semmle.code.java.regex.RegexDiffInformed import semmle.code.java.dataflow.FlowSources private import semmle.code.java.security.Sanitizers @@ -33,6 +34,14 @@ private class LengthRestrictedMethod extends Method { } } +class PolynomialRedDosDiffInformed extends RegexDiffInformedConfig { + override predicate observeDiffInformedIncrementalMode() { + not PolynomialRedosFlow::hasSourceInDiffRange() + } + + override Location getASelectedSinkLocation(DataFlow::Node sink) { result = sink.getLocation() } +} + /** A configuration for Polynomial ReDoS queries. */ module PolynomialRedosConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node src) { src instanceof ActiveThreatModelSource } diff --git a/java/ql/src/Security/CWE/CWE-020/OverlyLargeRange.ql b/java/ql/src/Security/CWE/CWE-020/OverlyLargeRange.ql index b8ea3e52dbd0..8d5cfab03055 100644 --- a/java/ql/src/Security/CWE/CWE-020/OverlyLargeRange.ql +++ b/java/ql/src/Security/CWE/CWE-020/OverlyLargeRange.ql @@ -12,9 +12,17 @@ * external/cwe/cwe-020 */ +import semmle.code.java.regex.RegexDiffInformed +import semmle.code.java.dataflow.DataFlow private import semmle.code.java.regex.RegexTreeView::RegexTreeView as TreeView import codeql.regex.OverlyLargeRangeQuery::Make +class OverlyLargeRangeDiffInformed extends RegexDiffInformedConfig { + override predicate observeDiffInformedIncrementalMode() { any() } + + override Location getASelectedSinkLocation(DataFlow::Node sink) { none() } +} + TreeView::RegExpCharacterClass potentialMisparsedCharClass() { // nested char classes are currently misparsed result.getAChild().(TreeView::RegExpNormalChar).getValue() = "[" diff --git a/java/ql/src/Security/CWE/CWE-1204/StaticInitializationVector.ql b/java/ql/src/Security/CWE/CWE-1204/StaticInitializationVector.ql index 258e0f871123..0ad9275899af 100644 --- a/java/ql/src/Security/CWE/CWE-1204/StaticInitializationVector.ql +++ b/java/ql/src/Security/CWE/CWE-1204/StaticInitializationVector.ql @@ -16,6 +16,8 @@ import semmle.code.java.security.StaticInitializationVectorQuery import StaticInitializationVectorFlow::PathGraph from StaticInitializationVectorFlow::PathNode source, StaticInitializationVectorFlow::PathNode sink -where StaticInitializationVectorFlow::flowPath(source, sink) +where + StaticInitializationVectorFlow::flowPath(source, sink) and + not arrayReachesUpdate(source.getNode().asExpr()) select sink.getNode(), source, sink, "A $@ should not be used for encryption.", source.getNode(), "static initialization vector" diff --git a/java/ql/src/Security/CWE/CWE-730/ReDoS.ql b/java/ql/src/Security/CWE/CWE-730/ReDoS.ql index ca4750fc8588..057da9fd3545 100644 --- a/java/ql/src/Security/CWE/CWE-730/ReDoS.ql +++ b/java/ql/src/Security/CWE/CWE-730/ReDoS.ql @@ -14,9 +14,17 @@ * external/cwe/cwe-400 */ +import semmle.code.java.regex.RegexDiffInformed +import semmle.code.java.dataflow.DataFlow private import semmle.code.java.regex.RegexTreeView::RegexTreeView as TreeView import codeql.regex.nfa.ExponentialBackTracking::Make as ExponentialBackTracking +class ReDoSDiffInformed extends RegexDiffInformedConfig { + override predicate observeDiffInformedIncrementalMode() { any() } + + override Location getASelectedSinkLocation(DataFlow::Node sink) { none() } +} + from TreeView::RegExpTerm t, string pump, ExponentialBackTracking::State s, string prefixMsg where ExponentialBackTracking::hasReDoSResult(t, pump, s, prefixMsg) and diff --git a/java/ql/test/query-tests/security/CWE-1204/StaticInitializationVector.expected b/java/ql/test/query-tests/security/CWE-1204/StaticInitializationVector.expected new file mode 100644 index 000000000000..1438764d7972 --- /dev/null +++ b/java/ql/test/query-tests/security/CWE-1204/StaticInitializationVector.expected @@ -0,0 +1,97 @@ +#select +| StaticInitializationVector.java:19:51:19:56 | ivSpec | StaticInitializationVector.java:13:21:13:81 | new byte[] : byte[] | StaticInitializationVector.java:19:51:19:56 | ivSpec | A $@ should not be used for encryption. | StaticInitializationVector.java:13:21:13:81 | new byte[] | static initialization vector | +| StaticInitializationVector.java:32:51:32:56 | ivSpec | StaticInitializationVector.java:26:21:26:32 | new byte[] : byte[] | StaticInitializationVector.java:32:51:32:56 | ivSpec | A $@ should not be used for encryption. | StaticInitializationVector.java:26:21:26:32 | new byte[] | static initialization vector | +| StaticInitializationVector.java:48:51:48:56 | ivSpec | StaticInitializationVector.java:39:21:39:32 | new byte[] : byte[] | StaticInitializationVector.java:48:51:48:56 | ivSpec | A $@ should not be used for encryption. | StaticInitializationVector.java:39:21:39:32 | new byte[] | static initialization vector | +| StaticInitializationVector.java:64:51:64:56 | ivSpec | StaticInitializationVector.java:55:30:58:9 | new byte[][] : byte[][] | StaticInitializationVector.java:64:51:64:56 | ivSpec | A $@ should not be used for encryption. | StaticInitializationVector.java:55:30:58:9 | new byte[][] | static initialization vector | +| StaticInitializationVector.java:80:51:80:56 | ivSpec | StaticInitializationVector.java:71:30:74:9 | new byte[][] : byte[][] | StaticInitializationVector.java:80:51:80:56 | ivSpec | A $@ should not be used for encryption. | StaticInitializationVector.java:71:30:74:9 | new byte[][] | static initialization vector | +| StaticInitializationVector.java:96:51:96:56 | ivSpec | StaticInitializationVector.java:88:13:88:23 | new byte[] : byte[] | StaticInitializationVector.java:96:51:96:56 | ivSpec | A $@ should not be used for encryption. | StaticInitializationVector.java:88:13:88:23 | new byte[] | static initialization vector | +| StaticInitializationVector.java:96:51:96:56 | ivSpec | StaticInitializationVector.java:89:13:89:24 | new byte[] : byte[] | StaticInitializationVector.java:96:51:96:56 | ivSpec | A $@ should not be used for encryption. | StaticInitializationVector.java:89:13:89:24 | new byte[] | static initialization vector | +edges +| StaticInitializationVector.java:13:21:13:81 | new byte[] : byte[] | StaticInitializationVector.java:15:61:15:62 | iv : byte[] | provenance | | +| StaticInitializationVector.java:15:35:15:63 | new GCMParameterSpec(...) : GCMParameterSpec | StaticInitializationVector.java:19:51:19:56 | ivSpec | provenance | Sink:MaD:45855 | +| StaticInitializationVector.java:15:61:15:62 | iv : byte[] | StaticInitializationVector.java:15:35:15:63 | new GCMParameterSpec(...) : GCMParameterSpec | provenance | MaD:45875 | +| StaticInitializationVector.java:26:21:26:32 | new byte[] : byte[] | StaticInitializationVector.java:28:61:28:62 | iv : byte[] | provenance | | +| StaticInitializationVector.java:28:35:28:63 | new GCMParameterSpec(...) : GCMParameterSpec | StaticInitializationVector.java:32:51:32:56 | ivSpec | provenance | Sink:MaD:45855 | +| StaticInitializationVector.java:28:61:28:62 | iv : byte[] | StaticInitializationVector.java:28:35:28:63 | new GCMParameterSpec(...) : GCMParameterSpec | provenance | MaD:45875 | +| StaticInitializationVector.java:39:21:39:32 | new byte[] : byte[] | StaticInitializationVector.java:44:54:44:55 | iv : byte[] | provenance | | +| StaticInitializationVector.java:44:34:44:56 | new IvParameterSpec(...) : IvParameterSpec | StaticInitializationVector.java:48:51:48:56 | ivSpec | provenance | Sink:MaD:45855 | +| StaticInitializationVector.java:44:54:44:55 | iv : byte[] | StaticInitializationVector.java:44:34:44:56 | new IvParameterSpec(...) : IvParameterSpec | provenance | MaD:45874 | +| StaticInitializationVector.java:55:30:58:9 | new byte[][] : byte[][] | StaticInitializationVector.java:60:61:60:72 | ...[...] : byte[] | provenance | | +| StaticInitializationVector.java:60:35:60:73 | new GCMParameterSpec(...) : GCMParameterSpec | StaticInitializationVector.java:64:51:64:56 | ivSpec | provenance | Sink:MaD:45855 | +| StaticInitializationVector.java:60:61:60:72 | ...[...] : byte[] | StaticInitializationVector.java:60:35:60:73 | new GCMParameterSpec(...) : GCMParameterSpec | provenance | MaD:45875 | +| StaticInitializationVector.java:71:30:74:9 | new byte[][] : byte[][] | StaticInitializationVector.java:76:61:76:72 | ...[...] : byte[] | provenance | | +| StaticInitializationVector.java:76:35:76:73 | new GCMParameterSpec(...) : GCMParameterSpec | StaticInitializationVector.java:80:51:80:56 | ivSpec | provenance | Sink:MaD:45855 | +| StaticInitializationVector.java:76:61:76:72 | ...[...] : byte[] | StaticInitializationVector.java:76:35:76:73 | new GCMParameterSpec(...) : GCMParameterSpec | provenance | MaD:45875 | +| StaticInitializationVector.java:87:24:90:9 | {...} : byte[][] [[]] : byte[] | StaticInitializationVector.java:92:61:92:63 | ivs : byte[][] [[]] : byte[] | provenance | | +| StaticInitializationVector.java:88:13:88:23 | new byte[] : byte[] | StaticInitializationVector.java:87:24:90:9 | {...} : byte[][] [[]] : byte[] | provenance | | +| StaticInitializationVector.java:89:13:89:24 | new byte[] : byte[] | StaticInitializationVector.java:87:24:90:9 | {...} : byte[][] [[]] : byte[] | provenance | | +| StaticInitializationVector.java:92:35:92:67 | new GCMParameterSpec(...) : GCMParameterSpec | StaticInitializationVector.java:96:51:96:56 | ivSpec | provenance | Sink:MaD:45855 | +| StaticInitializationVector.java:92:61:92:63 | ivs : byte[][] [[]] : byte[] | StaticInitializationVector.java:92:61:92:66 | ...[...] : byte[] | provenance | | +| StaticInitializationVector.java:92:61:92:66 | ...[...] : byte[] | StaticInitializationVector.java:92:35:92:67 | new GCMParameterSpec(...) : GCMParameterSpec | provenance | MaD:45875 | +| StaticInitializationVector.java:103:21:103:32 | new byte[] : byte[] | StaticInitializationVector.java:108:61:108:62 | iv : byte[] | provenance | | +| StaticInitializationVector.java:108:35:108:63 | new GCMParameterSpec(...) : GCMParameterSpec | StaticInitializationVector.java:112:51:112:56 | ivSpec | provenance | Sink:MaD:45855 | +| StaticInitializationVector.java:108:61:108:62 | iv : byte[] | StaticInitializationVector.java:108:35:108:63 | new GCMParameterSpec(...) : GCMParameterSpec | provenance | MaD:45875 | +| StaticInitializationVector.java:120:21:120:32 | new byte[] : byte[] | StaticInitializationVector.java:125:61:125:62 | iv : byte[] | provenance | | +| StaticInitializationVector.java:125:35:125:63 | new GCMParameterSpec(...) : GCMParameterSpec | StaticInitializationVector.java:129:51:129:56 | ivSpec | provenance | Sink:MaD:45855 | +| StaticInitializationVector.java:125:61:125:62 | iv : byte[] | StaticInitializationVector.java:125:35:125:63 | new GCMParameterSpec(...) : GCMParameterSpec | provenance | MaD:45875 | +| StaticInitializationVector.java:136:30:136:41 | new byte[] : byte[] | StaticInitializationVector.java:140:26:140:36 | randomBytes : byte[] | provenance | | +| StaticInitializationVector.java:139:21:139:32 | new byte[] : byte[] | StaticInitializationVector.java:142:61:142:62 | iv : byte[] | provenance | | +| StaticInitializationVector.java:140:26:140:36 | randomBytes : byte[] | StaticInitializationVector.java:140:42:140:43 | iv [post update] : byte[] | provenance | MaD:44199 | +| StaticInitializationVector.java:140:42:140:43 | iv [post update] : byte[] | StaticInitializationVector.java:142:61:142:62 | iv : byte[] | provenance | | +| StaticInitializationVector.java:142:35:142:63 | new GCMParameterSpec(...) : GCMParameterSpec | StaticInitializationVector.java:146:51:146:56 | ivSpec | provenance | Sink:MaD:45855 | +| StaticInitializationVector.java:142:61:142:62 | iv : byte[] | StaticInitializationVector.java:142:35:142:63 | new GCMParameterSpec(...) : GCMParameterSpec | provenance | MaD:45875 | +| StaticInitializationVector.java:172:30:172:43 | new byte[] : byte[] | StaticInitializationVector.java:174:16:174:26 | randomBytes : byte[] | provenance | | +| StaticInitializationVector.java:174:16:174:26 | randomBytes : byte[] | StaticInitializationVector.java:179:21:179:32 | generate(...) : byte[] | provenance | | +| StaticInitializationVector.java:179:21:179:32 | generate(...) : byte[] | StaticInitializationVector.java:181:54:181:55 | iv : byte[] | provenance | | +| StaticInitializationVector.java:181:34:181:56 | new IvParameterSpec(...) : IvParameterSpec | StaticInitializationVector.java:185:51:185:56 | ivSpec | provenance | Sink:MaD:45855 | +| StaticInitializationVector.java:181:54:181:55 | iv : byte[] | StaticInitializationVector.java:181:34:181:56 | new IvParameterSpec(...) : IvParameterSpec | provenance | MaD:45874 | +nodes +| StaticInitializationVector.java:13:21:13:81 | new byte[] : byte[] | semmle.label | new byte[] : byte[] | +| StaticInitializationVector.java:15:35:15:63 | new GCMParameterSpec(...) : GCMParameterSpec | semmle.label | new GCMParameterSpec(...) : GCMParameterSpec | +| StaticInitializationVector.java:15:61:15:62 | iv : byte[] | semmle.label | iv : byte[] | +| StaticInitializationVector.java:19:51:19:56 | ivSpec | semmle.label | ivSpec | +| StaticInitializationVector.java:26:21:26:32 | new byte[] : byte[] | semmle.label | new byte[] : byte[] | +| StaticInitializationVector.java:28:35:28:63 | new GCMParameterSpec(...) : GCMParameterSpec | semmle.label | new GCMParameterSpec(...) : GCMParameterSpec | +| StaticInitializationVector.java:28:61:28:62 | iv : byte[] | semmle.label | iv : byte[] | +| StaticInitializationVector.java:32:51:32:56 | ivSpec | semmle.label | ivSpec | +| StaticInitializationVector.java:39:21:39:32 | new byte[] : byte[] | semmle.label | new byte[] : byte[] | +| StaticInitializationVector.java:44:34:44:56 | new IvParameterSpec(...) : IvParameterSpec | semmle.label | new IvParameterSpec(...) : IvParameterSpec | +| StaticInitializationVector.java:44:54:44:55 | iv : byte[] | semmle.label | iv : byte[] | +| StaticInitializationVector.java:48:51:48:56 | ivSpec | semmle.label | ivSpec | +| StaticInitializationVector.java:55:30:58:9 | new byte[][] : byte[][] | semmle.label | new byte[][] : byte[][] | +| StaticInitializationVector.java:60:35:60:73 | new GCMParameterSpec(...) : GCMParameterSpec | semmle.label | new GCMParameterSpec(...) : GCMParameterSpec | +| StaticInitializationVector.java:60:61:60:72 | ...[...] : byte[] | semmle.label | ...[...] : byte[] | +| StaticInitializationVector.java:64:51:64:56 | ivSpec | semmle.label | ivSpec | +| StaticInitializationVector.java:71:30:74:9 | new byte[][] : byte[][] | semmle.label | new byte[][] : byte[][] | +| StaticInitializationVector.java:76:35:76:73 | new GCMParameterSpec(...) : GCMParameterSpec | semmle.label | new GCMParameterSpec(...) : GCMParameterSpec | +| StaticInitializationVector.java:76:61:76:72 | ...[...] : byte[] | semmle.label | ...[...] : byte[] | +| StaticInitializationVector.java:80:51:80:56 | ivSpec | semmle.label | ivSpec | +| StaticInitializationVector.java:87:24:90:9 | {...} : byte[][] [[]] : byte[] | semmle.label | {...} : byte[][] [[]] : byte[] | +| StaticInitializationVector.java:88:13:88:23 | new byte[] : byte[] | semmle.label | new byte[] : byte[] | +| StaticInitializationVector.java:89:13:89:24 | new byte[] : byte[] | semmle.label | new byte[] : byte[] | +| StaticInitializationVector.java:92:35:92:67 | new GCMParameterSpec(...) : GCMParameterSpec | semmle.label | new GCMParameterSpec(...) : GCMParameterSpec | +| StaticInitializationVector.java:92:61:92:63 | ivs : byte[][] [[]] : byte[] | semmle.label | ivs : byte[][] [[]] : byte[] | +| StaticInitializationVector.java:92:61:92:66 | ...[...] : byte[] | semmle.label | ...[...] : byte[] | +| StaticInitializationVector.java:96:51:96:56 | ivSpec | semmle.label | ivSpec | +| StaticInitializationVector.java:103:21:103:32 | new byte[] : byte[] | semmle.label | new byte[] : byte[] | +| StaticInitializationVector.java:108:35:108:63 | new GCMParameterSpec(...) : GCMParameterSpec | semmle.label | new GCMParameterSpec(...) : GCMParameterSpec | +| StaticInitializationVector.java:108:61:108:62 | iv : byte[] | semmle.label | iv : byte[] | +| StaticInitializationVector.java:112:51:112:56 | ivSpec | semmle.label | ivSpec | +| StaticInitializationVector.java:120:21:120:32 | new byte[] : byte[] | semmle.label | new byte[] : byte[] | +| StaticInitializationVector.java:125:35:125:63 | new GCMParameterSpec(...) : GCMParameterSpec | semmle.label | new GCMParameterSpec(...) : GCMParameterSpec | +| StaticInitializationVector.java:125:61:125:62 | iv : byte[] | semmle.label | iv : byte[] | +| StaticInitializationVector.java:129:51:129:56 | ivSpec | semmle.label | ivSpec | +| StaticInitializationVector.java:136:30:136:41 | new byte[] : byte[] | semmle.label | new byte[] : byte[] | +| StaticInitializationVector.java:139:21:139:32 | new byte[] : byte[] | semmle.label | new byte[] : byte[] | +| StaticInitializationVector.java:140:26:140:36 | randomBytes : byte[] | semmle.label | randomBytes : byte[] | +| StaticInitializationVector.java:140:42:140:43 | iv [post update] : byte[] | semmle.label | iv [post update] : byte[] | +| StaticInitializationVector.java:142:35:142:63 | new GCMParameterSpec(...) : GCMParameterSpec | semmle.label | new GCMParameterSpec(...) : GCMParameterSpec | +| StaticInitializationVector.java:142:61:142:62 | iv : byte[] | semmle.label | iv : byte[] | +| StaticInitializationVector.java:146:51:146:56 | ivSpec | semmle.label | ivSpec | +| StaticInitializationVector.java:172:30:172:43 | new byte[] : byte[] | semmle.label | new byte[] : byte[] | +| StaticInitializationVector.java:174:16:174:26 | randomBytes : byte[] | semmle.label | randomBytes : byte[] | +| StaticInitializationVector.java:179:21:179:32 | generate(...) : byte[] | semmle.label | generate(...) : byte[] | +| StaticInitializationVector.java:181:34:181:56 | new IvParameterSpec(...) : IvParameterSpec | semmle.label | new IvParameterSpec(...) : IvParameterSpec | +| StaticInitializationVector.java:181:54:181:55 | iv : byte[] | semmle.label | iv : byte[] | +| StaticInitializationVector.java:185:51:185:56 | ivSpec | semmle.label | ivSpec | +subpaths diff --git a/java/ql/test/query-tests/security/CWE-1204/StaticInitializationVector.java b/java/ql/test/query-tests/security/CWE-1204/StaticInitializationVector.java index dca6eb261ca1..4da986c4a3df 100644 --- a/java/ql/test/query-tests/security/CWE-1204/StaticInitializationVector.java +++ b/java/ql/test/query-tests/security/CWE-1204/StaticInitializationVector.java @@ -10,33 +10,33 @@ public class StaticInitializationVector { // BAD: AES-GCM with static IV from a byte array public byte[] encryptWithStaticIvByteArrayWithInitializer(byte[] key, byte[] plaintext) throws Exception { - byte[] iv = new byte[] { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0, 1, 2, 3, 4, 5 }; + byte[] iv = new byte[] { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0, 1, 2, 3, 4, 5 }; // $Source GCMParameterSpec ivSpec = new GCMParameterSpec(128, iv); SecretKeySpec keySpec = new SecretKeySpec(key, "AES"); Cipher cipher = Cipher.getInstance("AES/GCM/PKCS5PADDING"); - cipher.init(Cipher.ENCRYPT_MODE, keySpec, ivSpec); // $staticInitializationVector + cipher.init(Cipher.ENCRYPT_MODE, keySpec, ivSpec); // $Alert cipher.update(plaintext); return cipher.doFinal(); } // BAD: AES-GCM with static IV from zero-initialized byte array public byte[] encryptWithZeroStaticIvByteArray(byte[] key, byte[] plaintext) throws Exception { - byte[] iv = new byte[16]; + byte[] iv = new byte[16]; // $Source GCMParameterSpec ivSpec = new GCMParameterSpec(128, iv); SecretKeySpec keySpec = new SecretKeySpec(key, "AES"); Cipher cipher = Cipher.getInstance("AES/GCM/PKCS5PADDING"); - cipher.init(Cipher.ENCRYPT_MODE, keySpec, ivSpec); // $staticInitializationVector + cipher.init(Cipher.ENCRYPT_MODE, keySpec, ivSpec); // $Alert cipher.update(plaintext); return cipher.doFinal(); } // BAD: AES-CBC with static IV from zero-initialized byte array public byte[] encryptWithStaticIvByteArray(byte[] key, byte[] plaintext) throws Exception { - byte[] iv = new byte[16]; + byte[] iv = new byte[16]; // $Source for (byte i = 0; i < iv.length; i++) { iv[i] = 1; } @@ -45,7 +45,7 @@ public byte[] encryptWithStaticIvByteArray(byte[] key, byte[] plaintext) throws SecretKeySpec keySpec = new SecretKeySpec(key, "AES"); Cipher cipher = Cipher.getInstance("AES/CBC/PKCS5PADDING"); - cipher.init(Cipher.ENCRYPT_MODE, keySpec, ivSpec); // $staticInitializationVector + cipher.init(Cipher.ENCRYPT_MODE, keySpec, ivSpec); // $Alert cipher.update(plaintext); return cipher.doFinal(); } @@ -55,13 +55,13 @@ public byte[] encryptWithOneOfStaticIvs01(byte[] key, byte[] plaintext) throws E byte[][] staticIvs = new byte[][] { { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0, 1, 2, 3, 4, 5 }, { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0, 1, 2, 3, 4, 42 } - }; + }; // $Source GCMParameterSpec ivSpec = new GCMParameterSpec(128, staticIvs[1]); SecretKeySpec keySpec = new SecretKeySpec(key, "AES"); Cipher cipher = Cipher.getInstance("AES/GCM/PKCS5PADDING"); - cipher.init(Cipher.ENCRYPT_MODE, keySpec, ivSpec); // $staticInitializationVector + cipher.init(Cipher.ENCRYPT_MODE, keySpec, ivSpec); // $Alert cipher.update(plaintext); return cipher.doFinal(); } @@ -71,13 +71,13 @@ public byte[] encryptWithOneOfStaticIvs02(byte[] key, byte[] plaintext) throws E byte[][] staticIvs = new byte[][] { new byte[] { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0, 1, 2, 3, 4, 5 }, new byte[] { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0, 1, 2, 3, 4, 42 } - }; + }; // $Source GCMParameterSpec ivSpec = new GCMParameterSpec(128, staticIvs[1]); SecretKeySpec keySpec = new SecretKeySpec(key, "AES"); Cipher cipher = Cipher.getInstance("AES/GCM/PKCS5PADDING"); - cipher.init(Cipher.ENCRYPT_MODE, keySpec, ivSpec); // $staticInitializationVector + cipher.init(Cipher.ENCRYPT_MODE, keySpec, ivSpec); // $Alert cipher.update(plaintext); return cipher.doFinal(); } @@ -85,15 +85,15 @@ public byte[] encryptWithOneOfStaticIvs02(byte[] key, byte[] plaintext) throws E // BAD: AES-GCM with static IV from a multidimensional byte array public byte[] encryptWithOneOfStaticZeroIvs(byte[] key, byte[] plaintext) throws Exception { byte[][] ivs = new byte[][] { - new byte[8], - new byte[16] + new byte[8], // $Source + new byte[16] // $Source }; GCMParameterSpec ivSpec = new GCMParameterSpec(128, ivs[1]); SecretKeySpec keySpec = new SecretKeySpec(key, "AES"); Cipher cipher = Cipher.getInstance("AES/GCM/PKCS5PADDING"); - cipher.init(Cipher.ENCRYPT_MODE, keySpec, ivSpec); // $staticInitializationVector + cipher.init(Cipher.ENCRYPT_MODE, keySpec, ivSpec); // $Alert cipher.update(plaintext); return cipher.doFinal(); } diff --git a/java/ql/test/query-tests/security/CWE-1204/StaticInitializationVector.qlref b/java/ql/test/query-tests/security/CWE-1204/StaticInitializationVector.qlref new file mode 100644 index 000000000000..2686a2637cb5 --- /dev/null +++ b/java/ql/test/query-tests/security/CWE-1204/StaticInitializationVector.qlref @@ -0,0 +1,2 @@ +query: Security/CWE/CWE-1204/StaticInitializationVector.ql +postprocess: utils/test/InlineExpectationsTestQuery.ql diff --git a/java/ql/test/query-tests/security/CWE-1204/StaticInitializationVectorTest.expected b/java/ql/test/query-tests/security/CWE-1204/StaticInitializationVectorTest.expected deleted file mode 100644 index e69de29bb2d1..000000000000 diff --git a/java/ql/test/query-tests/security/CWE-1204/StaticInitializationVectorTest.ql b/java/ql/test/query-tests/security/CWE-1204/StaticInitializationVectorTest.ql deleted file mode 100644 index 5996cccdd4f4..000000000000 --- a/java/ql/test/query-tests/security/CWE-1204/StaticInitializationVectorTest.ql +++ /dev/null @@ -1,18 +0,0 @@ -import java -import semmle.code.java.security.StaticInitializationVectorQuery -import utils.test.InlineExpectationsTest - -module StaticInitializationVectorTest implements TestSig { - string getARelevantTag() { result = "staticInitializationVector" } - - predicate hasActualResult(Location location, string element, string tag, string value) { - tag = "staticInitializationVector" and - exists(DataFlow::Node sink | StaticInitializationVectorFlow::flowTo(sink) | - sink.getLocation() = location and - element = sink.toString() and - value = "" - ) - } -} - -import MakeTest diff --git a/shared/dataflow/codeql/dataflow/internal/DataFlowImpl.qll b/shared/dataflow/codeql/dataflow/internal/DataFlowImpl.qll index a13c71f554cc..989318691afc 100644 --- a/shared/dataflow/codeql/dataflow/internal/DataFlowImpl.qll +++ b/shared/dataflow/codeql/dataflow/internal/DataFlowImpl.qll @@ -8,6 +8,7 @@ private import codeql.util.Unit private import codeql.util.Option private import codeql.util.Boolean private import codeql.util.Location +private import codeql.util.AlertFiltering private import codeql.dataflow.DataFlow private import DataFlowImplStage1 @@ -19,6 +20,8 @@ module MakeImpl Lang> { private import DataFlowImplCommonPublic private import Aliases + private module AlertFiltering = AlertFilteringImpl; + /** * An input configuration for data flow using flow state. This signature equals * `StateConfigSig`, but requires explicit implementation of all predicates. @@ -3389,6 +3392,38 @@ module MakeImpl Lang> { */ predicate flowToExpr(Expr sink) { flowTo(exprNode(sink)) } + /** + * Holds if the configuration has at least one source in the diff range as + * determined by `AlertFiltering`. This predicate is independent of whether + * diff-informed mode is observed by the configuration and is also + * independent whether there was flow. + */ + pragma[nomagic] + predicate hasSourceInDiffRange() { + exists(Node source | + Config::isSource(source, _) and + AlertFiltering::filterByLocation(Config::getASelectedSourceLocation(source)) + // TODO: also require `Config::isSink(_, _)`? Because if the source + // isn't reachable, it may as well not be there. + ) + } + + /** + * Holds if the configuration has at least one sink in the diff range as + * determined by `AlertFiltering`. This predicate is independent of whether + * diff-informed mode is observed by the configuration and is also + * independent whether there was flow. + */ + pragma[nomagic] + predicate hasSinkInDiffRange() { + exists(Node sink | + Config::isSink(sink, _) and + AlertFiltering::filterByLocation(Config::getASelectedSinkLocation(sink)) + // TODO: also require `Config::isSource(_, _)`? Because if the sink + // isn't reachable, it may as well not be there. + ) + } + /** * INTERNAL: Only for debugging. *