diff --git a/java/ql/lib/semmle/code/java/security/RequestForgeryConfig.qll b/java/ql/lib/semmle/code/java/security/RequestForgeryConfig.qll index c8a59f65b0e..ed5adb81216 100644 --- a/java/ql/lib/semmle/code/java/security/RequestForgeryConfig.qll +++ b/java/ql/lib/semmle/code/java/security/RequestForgeryConfig.qll @@ -8,9 +8,11 @@ import semmle.code.java.dataflow.FlowSources import semmle.code.java.security.RequestForgery /** + * DEPRECATED: Use `RequestForgeryConfiguration` module instead. + * * A taint-tracking configuration characterising request-forgery risks. */ -class RequestForgeryConfiguration extends TaintTracking::Configuration { +deprecated class RequestForgeryConfiguration extends TaintTracking::Configuration { RequestForgeryConfiguration() { this = "Server-Side Request Forgery" } override predicate isSource(DataFlow::Node source) { @@ -29,3 +31,26 @@ class RequestForgeryConfiguration extends TaintTracking::Configuration { override predicate isSanitizer(DataFlow::Node node) { node instanceof RequestForgerySanitizer } } + +/** + * A taint-tracking configuration characterising request-forgery risks. + */ +private module RequestForgeryConfiguration implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { + source instanceof RemoteFlowSource and + // Exclude results of remote HTTP requests: fetching something else based on that result + // is no worse than following a redirect returned by the remote server, and typically + // we're requesting a resource via https which we trust to only send us to safe URLs. + not source.asExpr().(MethodAccess).getCallee() instanceof UrlConnectionGetInputStreamMethod + } + + predicate isSink(DataFlow::Node sink) { sink instanceof RequestForgerySink } + + predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) { + any(RequestForgeryAdditionalTaintStep r).propagatesTaint(pred, succ) + } + + predicate isBarrier(DataFlow::Node node) { node instanceof RequestForgerySanitizer } +} + +module RequestForgeryFlow = TaintTracking::Make; diff --git a/java/ql/lib/semmle/code/java/security/SensitiveLoggingQuery.qll b/java/ql/lib/semmle/code/java/security/SensitiveLoggingQuery.qll index 1956360e120..c0dd06b2bc9 100644 --- a/java/ql/lib/semmle/code/java/security/SensitiveLoggingQuery.qll +++ b/java/ql/lib/semmle/code/java/security/SensitiveLoggingQuery.qll @@ -25,8 +25,12 @@ private class TypeType extends RefType { } } -/** A data-flow configuration for identifying potentially-sensitive data flowing to a log output. */ -class SensitiveLoggerConfiguration extends TaintTracking::Configuration { +/** + * DEPRECATED: Use `SensitiveLoggerConfiguration` module instead. + * + * A data-flow configuration for identifying potentially-sensitive data flowing to a log output. + */ +deprecated class SensitiveLoggerConfiguration extends TaintTracking::Configuration { SensitiveLoggerConfiguration() { this = "SensitiveLoggerConfiguration" } override predicate isSource(DataFlow::Node source) { source.asExpr() instanceof CredentialExpr } @@ -43,3 +47,22 @@ class SensitiveLoggerConfiguration extends TaintTracking::Configuration { override predicate isSanitizerIn(Node node) { this.isSource(node) } } + +/** A data-flow configuration for identifying potentially-sensitive data flowing to a log output. */ +private module SensitiveLoggerConfiguration implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source.asExpr() instanceof CredentialExpr } + + predicate isSink(DataFlow::Node sink) { sinkNode(sink, "logging") } + + predicate isBarrier(DataFlow::Node sanitizer) { + sanitizer.asExpr() instanceof LiveLiteral or + sanitizer.getType() instanceof PrimitiveType or + sanitizer.getType() instanceof BoxedType or + sanitizer.getType() instanceof NumberType or + sanitizer.getType() instanceof TypeType + } + + predicate isBarrierIn(Node node) { isSource(node) } +} + +module SensitiveLoggerFlow = TaintTracking::Make; diff --git a/java/ql/lib/semmle/code/java/security/XSS.qll b/java/ql/lib/semmle/code/java/security/XSS.qll index 9d15d8edeb5..9d03a81ae1e 100644 --- a/java/ql/lib/semmle/code/java/security/XSS.qll +++ b/java/ql/lib/semmle/code/java/security/XSS.qll @@ -7,7 +7,7 @@ import semmle.code.java.frameworks.spring.SpringController import semmle.code.java.frameworks.spring.SpringHttp import semmle.code.java.frameworks.javaee.jsf.JSFRenderer import semmle.code.java.dataflow.DataFlow -import semmle.code.java.dataflow.TaintTracking2 +import semmle.code.java.dataflow.TaintTracking private import semmle.code.java.dataflow.ExternalFlow /** A sink that represent a method that outputs data without applying contextual output encoding. */ @@ -41,9 +41,9 @@ private class DefaultXssSink extends XssSink { DefaultXssSink() { sinkNode(this, "xss") or - exists(XssVulnerableWriterSourceToWritingMethodFlowConfig writer, MethodAccess ma | + exists(MethodAccess ma | ma.getMethod() instanceof WritingMethod and - writer.hasFlowToExpr(ma.getQualifier()) and + XssVulnerableWriterSourceToWritingMethodFlow::hasFlowToExpr(ma.getQualifier()) and this.asExpr() = ma.getArgument(_) ) } @@ -60,23 +60,19 @@ private class DefaultXssSanitizer extends XssSanitizer { } /** A configuration that tracks data from a servlet writer to an output method. */ -private class XssVulnerableWriterSourceToWritingMethodFlowConfig extends TaintTracking2::Configuration -{ - XssVulnerableWriterSourceToWritingMethodFlowConfig() { - this = "XSS::XssVulnerableWriterSourceToWritingMethodFlowConfig" - } +private module XssVulnerableWriterSourceToWritingMethodFlowConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node src) { src.asExpr() instanceof XssVulnerableWriterSource } - override predicate isSource(DataFlow::Node src) { - src.asExpr() instanceof XssVulnerableWriterSource - } - - override predicate isSink(DataFlow::Node sink) { + predicate isSink(DataFlow::Node sink) { exists(MethodAccess ma | sink.asExpr() = ma.getQualifier() and ma.getMethod() instanceof WritingMethod ) } } +private module XssVulnerableWriterSourceToWritingMethodFlow = + TaintTracking::Make; + /** A method that can be used to output data to an output stream or writer. */ private class WritingMethod extends Method { WritingMethod() { diff --git a/java/ql/src/Security/CWE/CWE-022/TaintedPath.ql b/java/ql/src/Security/CWE/CWE-022/TaintedPath.ql index e64059b63d6..c7c3ce0f835 100644 --- a/java/ql/src/Security/CWE/CWE-022/TaintedPath.ql +++ b/java/ql/src/Security/CWE/CWE-022/TaintedPath.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 TaintedPathConfig extends TaintTracking::Configuration { - TaintedPathConfig() { this = "TaintedPathConfig" } +module TaintedPathConfig 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(PathCreation p).getAnInput() or sinkNode(sink, ["create-file", "read-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 TaintedPath = TaintTracking::Make; + +import TaintedPath::PathGraph + /** * Gets the data-flow node at which to report a path ending at `sink`. * @@ -52,13 +53,13 @@ class TaintedPathConfig extends TaintTracking::Configuration { * continue to report there; otherwise we report directly at `sink`. */ DataFlow::Node getReportingNode(DataFlow::Node sink) { - any(TaintedPathConfig c).hasFlowTo(sink) and + TaintedPath::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, TaintedPathConfig conf -where conf.hasFlowPath(source, sink) +from TaintedPath::PathNode source, TaintedPath::PathNode sink +where TaintedPath::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-079/XSS.ql b/java/ql/src/Security/CWE/CWE-079/XSS.ql index fe071334c48..f2b0a65f9fe 100644 --- a/java/ql/src/Security/CWE/CWE-079/XSS.ql +++ b/java/ql/src/Security/CWE/CWE-079/XSS.ql @@ -14,25 +14,26 @@ import java import semmle.code.java.dataflow.FlowSources import semmle.code.java.security.XSS -import DataFlow::PathGraph -class XssConfig extends TaintTracking::Configuration { - XssConfig() { this = "XSSConfig" } +module XssConfig 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 XssSink } - override predicate isSink(DataFlow::Node sink) { sink instanceof XssSink } + predicate isBarrier(DataFlow::Node node) { node instanceof XssSanitizer } - override predicate isSanitizer(DataFlow::Node node) { node instanceof XssSanitizer } + predicate isBarrierOut(DataFlow::Node node) { node instanceof XssSinkBarrier } - override predicate isSanitizerOut(DataFlow::Node node) { node instanceof XssSinkBarrier } - - override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) { + predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { any(XssAdditionalTaintStep s).step(node1, node2) } } -from DataFlow::PathNode source, DataFlow::PathNode sink, XssConfig conf -where conf.hasFlowPath(source, sink) +module XssFlow = TaintTracking::Make; + +import XssFlow::PathGraph + +from XssFlow::PathNode source, XssFlow::PathNode sink +where XssFlow::hasFlowPath(source, sink) select sink.getNode(), source, sink, "Cross-site scripting vulnerability due to a $@.", source.getNode(), "user-provided value" diff --git a/java/ql/src/Security/CWE/CWE-113/ResponseSplitting.ql b/java/ql/src/Security/CWE/CWE-113/ResponseSplitting.ql index 641ac5399a0..b1682dd5774 100644 --- a/java/ql/src/Security/CWE/CWE-113/ResponseSplitting.ql +++ b/java/ql/src/Security/CWE/CWE-113/ResponseSplitting.ql @@ -14,19 +14,16 @@ import java import semmle.code.java.dataflow.FlowSources import semmle.code.java.security.ResponseSplitting -import DataFlow::PathGraph -class ResponseSplittingConfig extends TaintTracking::Configuration { - ResponseSplittingConfig() { this = "ResponseSplittingConfig" } - - override predicate isSource(DataFlow::Node source) { +module ResponseSplittingConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource and not source instanceof SafeHeaderSplittingSource } - override predicate isSink(DataFlow::Node sink) { sink instanceof HeaderSplittingSink } + predicate isSink(DataFlow::Node sink) { sink instanceof HeaderSplittingSink } - override predicate isSanitizer(DataFlow::Node node) { + predicate isBarrier(DataFlow::Node node) { node.getType() instanceof PrimitiveType or node.getType() instanceof BoxedType @@ -45,8 +42,12 @@ class ResponseSplittingConfig extends TaintTracking::Configuration { } } -from DataFlow::PathNode source, DataFlow::PathNode sink, ResponseSplittingConfig conf -where conf.hasFlowPath(source, sink) +module ResponseSplitting = TaintTracking::Make; + +import ResponseSplitting::PathGraph + +from ResponseSplitting::PathNode source, ResponseSplitting::PathNode sink +where ResponseSplitting::hasFlowPath(source, sink) select sink.getNode(), source, sink, "This header depends on a $@, which may cause a response-splitting vulnerability.", source.getNode(), "user-provided value" diff --git a/java/ql/src/Security/CWE/CWE-113/ResponseSplittingLocal.ql b/java/ql/src/Security/CWE/CWE-113/ResponseSplittingLocal.ql index 89af15339d0..321f5659e27 100644 --- a/java/ql/src/Security/CWE/CWE-113/ResponseSplittingLocal.ql +++ b/java/ql/src/Security/CWE/CWE-113/ResponseSplittingLocal.ql @@ -14,23 +14,24 @@ import java import semmle.code.java.dataflow.FlowSources import semmle.code.java.security.ResponseSplitting -import DataFlow::PathGraph -class ResponseSplittingLocalConfig extends TaintTracking::Configuration { - ResponseSplittingLocalConfig() { this = "ResponseSplittingLocalConfig" } +module ResponseSplittingLocalConfig 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) { sink instanceof HeaderSplittingSink } - override predicate isSink(DataFlow::Node sink) { sink instanceof HeaderSplittingSink } - - override predicate isSanitizer(DataFlow::Node node) { + predicate isBarrier(DataFlow::Node node) { node.getType() instanceof PrimitiveType or node.getType() instanceof BoxedType } } -from DataFlow::PathNode source, DataFlow::PathNode sink, ResponseSplittingLocalConfig conf -where conf.hasFlowPath(source, sink) +module ResponseSplitting = TaintTracking::Make; + +import ResponseSplitting::PathGraph + +from ResponseSplitting::PathNode source, ResponseSplitting::PathNode sink +where ResponseSplitting::hasFlowPath(source, sink) select sink.getNode(), source, sink, "This header depends on a $@, which may cause a response-splitting vulnerability.", source.getNode(), "user-provided value" diff --git a/java/ql/src/Security/CWE/CWE-209/StackTraceExposure.ql b/java/ql/src/Security/CWE/CWE-209/StackTraceExposure.ql index c7da4c0e1f6..02934e98840 100644 --- a/java/ql/src/Security/CWE/CWE-209/StackTraceExposure.ql +++ b/java/ql/src/Security/CWE/CWE-209/StackTraceExposure.ql @@ -31,33 +31,27 @@ class PrintStackTraceMethod extends Method { } } -class ServletWriterSourceToPrintStackTraceMethodFlowConfig extends TaintTracking::Configuration { - ServletWriterSourceToPrintStackTraceMethodFlowConfig() { - this = "StackTraceExposure::ServletWriterSourceToPrintStackTraceMethodFlowConfig" - } +module ServletWriterSourceToPrintStackTraceMethodFlowConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node src) { src.asExpr() instanceof XssVulnerableWriterSource } - override predicate isSource(DataFlow::Node src) { - src.asExpr() instanceof XssVulnerableWriterSource - } - - override predicate isSink(DataFlow::Node sink) { + predicate isSink(DataFlow::Node sink) { exists(MethodAccess ma | sink.asExpr() = ma.getAnArgument() and ma.getMethod() instanceof PrintStackTraceMethod ) } } +module ServletWriterSourceToPrintStackTraceMethodFlow = + TaintTracking::Make; + /** * A call that uses `Throwable.printStackTrace()` on a stream that is connected * to external output. */ predicate printsStackToWriter(MethodAccess call) { - exists( - ServletWriterSourceToPrintStackTraceMethodFlowConfig writerSource, - PrintStackTraceMethod printStackTrace - | + exists(PrintStackTraceMethod printStackTrace | call.getMethod() = printStackTrace and - writerSource.hasFlowToExpr(call.getAnArgument()) + ServletWriterSourceToPrintStackTraceMethodFlow::hasFlowToExpr(call.getAnArgument()) ) } @@ -86,16 +80,15 @@ predicate stackTraceExpr(Expr exception, MethodAccess stackTraceString) { ) } -class StackTraceStringToHttpResponseSinkFlowConfig extends TaintTracking::Configuration { - StackTraceStringToHttpResponseSinkFlowConfig() { - this = "StackTraceExposure::StackTraceStringToHttpResponseSinkFlowConfig" - } +module StackTraceStringToHttpResponseSinkFlowConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node src) { stackTraceExpr(_, src.asExpr()) } - override predicate isSource(DataFlow::Node src) { stackTraceExpr(_, src.asExpr()) } - - override predicate isSink(DataFlow::Node sink) { sink instanceof InformationLeakSink } + predicate isSink(DataFlow::Node sink) { sink instanceof InformationLeakSink } } +module StackTraceStringToHttpResponseSinkFlow = + TaintTracking::Make; + /** * A write of stack trace data to an external stream. */ @@ -109,9 +102,10 @@ predicate printsStackExternally(MethodAccess call, Expr stackTrace) { * A stringified stack trace flows to an external sink. */ predicate stringifiedStackFlowsExternally(DataFlow::Node externalExpr, Expr stackTrace) { - exists(MethodAccess stackTraceString, StackTraceStringToHttpResponseSinkFlowConfig conf | + exists(MethodAccess stackTraceString | stackTraceExpr(stackTrace, stackTraceString) and - conf.hasFlow(DataFlow::exprNode(stackTraceString), externalExpr) + StackTraceStringToHttpResponseSinkFlow::hasFlow(DataFlow::exprNode(stackTraceString), + externalExpr) ) } diff --git a/java/ql/src/Security/CWE/CWE-327/BrokenCryptoAlgorithm.ql b/java/ql/src/Security/CWE/CWE-327/BrokenCryptoAlgorithm.ql index 3fa4b63015d..a68d43aec3f 100644 --- a/java/ql/src/Security/CWE/CWE-327/BrokenCryptoAlgorithm.ql +++ b/java/ql/src/Security/CWE/CWE-327/BrokenCryptoAlgorithm.ql @@ -15,7 +15,6 @@ import java import semmle.code.java.security.Encryption import semmle.code.java.dataflow.TaintTracking import DataFlow -import PathGraph private class ShortStringLiteral extends StringLiteral { ShortStringLiteral() { getValue().length() < 100 } @@ -29,24 +28,26 @@ class BrokenAlgoLiteral extends ShortStringLiteral { } } -class InsecureCryptoConfiguration extends TaintTracking::Configuration { - InsecureCryptoConfiguration() { this = "BrokenCryptoAlgortihm::InsecureCryptoConfiguration" } +module InsecureCryptoConfiguration implements ConfigSig { + predicate isSource(Node n) { n.asExpr() instanceof BrokenAlgoLiteral } - override predicate isSource(Node n) { n.asExpr() instanceof BrokenAlgoLiteral } + predicate isSink(Node n) { exists(CryptoAlgoSpec c | n.asExpr() = c.getAlgoSpec()) } - override predicate isSink(Node n) { exists(CryptoAlgoSpec c | n.asExpr() = c.getAlgoSpec()) } - - override predicate isSanitizer(DataFlow::Node node) { + predicate isBarrier(DataFlow::Node node) { node.getType() instanceof PrimitiveType or node.getType() instanceof BoxedType } } +module InsecureCryptoFlow = TaintTracking::Make; + +import InsecureCryptoFlow::PathGraph + from - PathNode source, PathNode sink, CryptoAlgoSpec c, BrokenAlgoLiteral s, - InsecureCryptoConfiguration conf + InsecureCryptoFlow::PathNode source, InsecureCryptoFlow::PathNode sink, CryptoAlgoSpec c, + BrokenAlgoLiteral s where sink.getNode().asExpr() = c.getAlgoSpec() and source.getNode().asExpr() = s and - conf.hasFlowPath(source, sink) + InsecureCryptoFlow::hasFlowPath(source, sink) select c, source, sink, "Cryptographic algorithm $@ is weak and should not be used.", s, s.getValue() diff --git a/java/ql/src/Security/CWE/CWE-327/MaybeBrokenCryptoAlgorithm.ql b/java/ql/src/Security/CWE/CWE-327/MaybeBrokenCryptoAlgorithm.ql index 49de716b1dc..e1d071dad0a 100644 --- a/java/ql/src/Security/CWE/CWE-327/MaybeBrokenCryptoAlgorithm.ql +++ b/java/ql/src/Security/CWE/CWE-327/MaybeBrokenCryptoAlgorithm.ql @@ -16,7 +16,6 @@ import semmle.code.java.security.Encryption import semmle.code.java.dataflow.TaintTracking import DataFlow import semmle.code.java.dispatch.VirtualDispatch -import PathGraph private class ShortStringLiteral extends StringLiteral { ShortStringLiteral() { this.getValue().length() < 100 } @@ -51,26 +50,28 @@ class StringContainer extends RefType { } } -class InsecureCryptoConfiguration extends TaintTracking::Configuration { - InsecureCryptoConfiguration() { this = "InsecureCryptoConfiguration" } +module InsecureCryptoConfiguration implements ConfigSig { + predicate isSource(Node n) { n.asExpr() instanceof InsecureAlgoLiteral } - override predicate isSource(Node n) { n.asExpr() instanceof InsecureAlgoLiteral } + predicate isSink(Node n) { exists(CryptoAlgoSpec c | n.asExpr() = c.getAlgoSpec()) } - override predicate isSink(Node n) { exists(CryptoAlgoSpec c | n.asExpr() = c.getAlgoSpec()) } - - override predicate isSanitizer(Node n) { + predicate isBarrier(Node n) { objectToString(n.asExpr()) or not n.getType().getErasure() instanceof StringContainer } } +module InsecureCryptoFlow = TaintTracking::Make; + +import InsecureCryptoFlow::PathGraph + from - PathNode source, PathNode sink, CryptoAlgoSpec c, InsecureAlgoLiteral s, - InsecureCryptoConfiguration conf + InsecureCryptoFlow::PathNode source, InsecureCryptoFlow::PathNode sink, CryptoAlgoSpec c, + InsecureAlgoLiteral s where sink.getNode().asExpr() = c.getAlgoSpec() and source.getNode().asExpr() = s and - conf.hasFlowPath(source, sink) + InsecureCryptoFlow::hasFlowPath(source, sink) select c, source, sink, "Cryptographic algorithm $@ may not be secure, consider using a different algorithm.", s, s.getValue() diff --git a/java/ql/src/Security/CWE/CWE-532/SensitiveInfoLog.ql b/java/ql/src/Security/CWE/CWE-532/SensitiveInfoLog.ql index 6f32b9768cb..a884ba6c242 100644 --- a/java/ql/src/Security/CWE/CWE-532/SensitiveInfoLog.ql +++ b/java/ql/src/Security/CWE/CWE-532/SensitiveInfoLog.ql @@ -13,9 +13,9 @@ import java import semmle.code.java.security.SensitiveLoggingQuery -import PathGraph +import SensitiveLoggerFlow::PathGraph -from SensitiveLoggerConfiguration cfg, DataFlow::PathNode source, DataFlow::PathNode sink -where cfg.hasFlowPath(source, sink) +from SensitiveLoggerFlow::PathNode source, SensitiveLoggerFlow::PathNode sink +where SensitiveLoggerFlow::hasFlowPath(source, sink) select sink.getNode(), source, sink, "This $@ is written to a log file.", source.getNode(), "potentially sensitive information" diff --git a/java/ql/src/Security/CWE/CWE-681/NumericCastTainted.ql b/java/ql/src/Security/CWE/CWE-681/NumericCastTainted.ql index 87972319903..3caefe3fd09 100644 --- a/java/ql/src/Security/CWE/CWE-681/NumericCastTainted.ql +++ b/java/ql/src/Security/CWE/CWE-681/NumericCastTainted.ql @@ -15,19 +15,16 @@ import java import semmle.code.java.dataflow.FlowSources import NumericCastCommon -import DataFlow::PathGraph -private class NumericCastFlowConfig extends TaintTracking::Configuration { - NumericCastFlowConfig() { this = "NumericCastTainted::RemoteUserInputToNumericNarrowingCastExpr" } +module NumericCastFlowConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node src) { src instanceof RemoteFlowSource } - override predicate isSource(DataFlow::Node src) { src instanceof RemoteFlowSource } - - override predicate isSink(DataFlow::Node sink) { + predicate isSink(DataFlow::Node sink) { sink.asExpr() = any(NumericNarrowingCastExpr cast).getExpr() and sink.asExpr() instanceof VarAccess } - override predicate isSanitizer(DataFlow::Node node) { + predicate isBarrier(DataFlow::Node node) { boundedRead(node.asExpr()) or castCheck(node.asExpr()) or node.getType() instanceof SmallType or @@ -37,12 +34,14 @@ private class NumericCastFlowConfig extends TaintTracking::Configuration { } } -from - DataFlow::PathNode source, DataFlow::PathNode sink, NumericNarrowingCastExpr exp, - NumericCastFlowConfig conf +module NumericCastFlow = TaintTracking::Make; + +import NumericCastFlow::PathGraph + +from NumericCastFlow::PathNode source, NumericCastFlow::PathNode sink, NumericNarrowingCastExpr exp where sink.getNode().asExpr() = exp.getExpr() and - conf.hasFlowPath(source, sink) + NumericCastFlow::hasFlowPath(source, sink) select exp, source, sink, "This cast to a narrower type depends on a $@, potentially causing truncation.", source.getNode(), "user-provided value" diff --git a/java/ql/src/Security/CWE/CWE-681/NumericCastTaintedLocal.ql b/java/ql/src/Security/CWE/CWE-681/NumericCastTaintedLocal.ql index 7ebbd73c84e..41757ab419a 100644 --- a/java/ql/src/Security/CWE/CWE-681/NumericCastTaintedLocal.ql +++ b/java/ql/src/Security/CWE/CWE-681/NumericCastTaintedLocal.ql @@ -15,20 +15,15 @@ import java import semmle.code.java.dataflow.FlowSources import NumericCastCommon -import DataFlow::PathGraph -private class NumericCastFlowConfig extends TaintTracking::Configuration { - NumericCastFlowConfig() { - this = "NumericCastTaintedLocal::LocalUserInputToNumericNarrowingCastExpr" - } +module NumericCastFlowConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node src) { src instanceof LocalUserInput } - override predicate isSource(DataFlow::Node src) { src instanceof LocalUserInput } - - override predicate isSink(DataFlow::Node sink) { + predicate isSink(DataFlow::Node sink) { sink.asExpr() = any(NumericNarrowingCastExpr cast).getExpr() } - override predicate isSanitizer(DataFlow::Node node) { + predicate isBarrier(DataFlow::Node node) { boundedRead(node.asExpr()) or castCheck(node.asExpr()) or node.getType() instanceof SmallType or @@ -37,13 +32,17 @@ private class NumericCastFlowConfig extends TaintTracking::Configuration { } } +module NumericCastFlow = TaintTracking::Make; + +import NumericCastFlow::PathGraph + from - DataFlow::PathNode source, DataFlow::PathNode sink, NumericNarrowingCastExpr exp, - VarAccess tainted, NumericCastFlowConfig conf + NumericCastFlow::PathNode source, NumericCastFlow::PathNode sink, NumericNarrowingCastExpr exp, + VarAccess tainted where exp.getExpr() = tainted and sink.getNode().asExpr() = tainted and - conf.hasFlowPath(source, sink) and + NumericCastFlow::hasFlowPath(source, sink) and not exists(RightShiftOp e | e.getShiftedVariable() = tainted.getVariable()) select exp, source, sink, "This cast to a narrower type depends on a $@, potentially causing truncation.", source.getNode(), diff --git a/java/ql/src/Security/CWE/CWE-918/RequestForgery.ql b/java/ql/src/Security/CWE/CWE-918/RequestForgery.ql index 604dd366513..c3455dc4beb 100644 --- a/java/ql/src/Security/CWE/CWE-918/RequestForgery.ql +++ b/java/ql/src/Security/CWE/CWE-918/RequestForgery.ql @@ -13,9 +13,9 @@ import java import semmle.code.java.security.RequestForgeryConfig -import DataFlow::PathGraph +import RequestForgeryFlow::PathGraph -from DataFlow::PathNode source, DataFlow::PathNode sink, RequestForgeryConfiguration conf -where conf.hasFlowPath(source, sink) +from RequestForgeryFlow::PathNode source, RequestForgeryFlow::PathNode sink +where RequestForgeryFlow::hasFlowPath(source, sink) select sink.getNode(), source, sink, "Potential server-side request forgery due to a $@.", source.getNode(), "user-provided value" diff --git a/java/ql/test/query-tests/security/CWE-532/SensitiveLogInfo.ql b/java/ql/test/query-tests/security/CWE-532/SensitiveLogInfo.ql index 02cdfacc56f..b868dcabf85 100644 --- a/java/ql/test/query-tests/security/CWE-532/SensitiveLogInfo.ql +++ b/java/ql/test/query-tests/security/CWE-532/SensitiveLogInfo.ql @@ -2,14 +2,10 @@ import java import TestUtilities.InlineFlowTest import semmle.code.java.security.SensitiveLoggingQuery -class EnableLegacy extends EnableLegacyConfiguration { - EnableLegacy() { exists(this) } -} - class HasFlowTest extends InlineFlowTest { - override DataFlow::Configuration getTaintFlowConfig() { - result instanceof SensitiveLoggerConfiguration + override predicate hasTaintFlow(DataFlow::Node src, DataFlow::Node sink) { + SensitiveLoggerFlow::hasFlow(src, sink) } - override DataFlow::Configuration getValueFlowConfig() { none() } + override predicate hasValueFlow(DataFlow::Node src, DataFlow::Node sink) { none() } } diff --git a/java/ql/test/query-tests/security/CWE-918/RequestForgery.ql b/java/ql/test/query-tests/security/CWE-918/RequestForgery.ql index d7e481ce618..b27d81ee073 100644 --- a/java/ql/test/query-tests/security/CWE-918/RequestForgery.ql +++ b/java/ql/test/query-tests/security/CWE-918/RequestForgery.ql @@ -9,7 +9,8 @@ class HasFlowTest extends InlineExpectationsTest { override predicate hasActualResult(Location location, string element, string tag, string value) { tag = "SSRF" and - exists(RequestForgeryConfiguration conf, DataFlow::Node sink | conf.hasFlowTo(sink) | + exists(DataFlow::Node sink | + RequestForgeryFlow::hasFlowTo(sink) and sink.getLocation() = location and element = sink.toString() and value = ""