From c14a4c4d930a1b4896f470df7a8302af30daea63 Mon Sep 17 00:00:00 2001 From: tiferet Date: Tue, 27 Dec 2022 00:10:31 -0800 Subject: [PATCH] Add an implementation of TaintedPathATM.qll and corresponding positive EndpointCharacteristic in Java --- .../EndpointCharacteristics.qll | 48 ++++++----- .../adaptivethreatmodeling/TaintedPathATM.qll | 80 ++++++++++++------- .../src/ExtractPositiveExamples.ql | 3 +- .../src/ExtractSinkCandidatesWithFlow.ql | 28 ++++--- 4 files changed, 96 insertions(+), 63 deletions(-) diff --git a/java/ql/experimental/adaptivethreatmodeling/lib/experimental/adaptivethreatmodeling/EndpointCharacteristics.qll b/java/ql/experimental/adaptivethreatmodeling/lib/experimental/adaptivethreatmodeling/EndpointCharacteristics.qll index 88eb5ad59e2..2b9cd8379b2 100644 --- a/java/ql/experimental/adaptivethreatmodeling/lib/experimental/adaptivethreatmodeling/EndpointCharacteristics.qll +++ b/java/ql/experimental/adaptivethreatmodeling/lib/experimental/adaptivethreatmodeling/EndpointCharacteristics.qll @@ -5,9 +5,12 @@ private import java as java import semmle.code.java.dataflow.TaintTracking import semmle.code.java.security.QueryInjection +import semmle.code.java.security.PathCreation +private import semmle.code.java.dataflow.ExternalFlow import experimental.adaptivethreatmodeling.EndpointTypes private import experimental.adaptivethreatmodeling.ATMConfig private import experimental.adaptivethreatmodeling.SqlInjectionATM +private import experimental.adaptivethreatmodeling.TaintedPathATM private import semmle.code.java.security.ExternalAPIs as ExternalAPIs private import semmle.code.java.Expr as Expr @@ -155,11 +158,11 @@ abstract class EndpointCharacteristic extends string { /* * Characteristics that are indicative of a sink. * NOTE: Initially each sink type has only one characteristic, which is that it's a sink of this type in the standard - * JavaScript libraries. + * Java libraries. */ // /** -// * Endpoints identified as "DomBasedXssSink" by the standard JavaScript libraries are XSS sinks with maximal confidence. +// * Endpoints identified as "DomBasedXssSink" by the standard Java libraries are XSS sinks with maximal confidence. // */ // private class DomBasedXssSinkCharacteristic extends EndpointCharacteristic { // DomBasedXssSinkCharacteristic() { this = "DomBasedXssSink" } @@ -172,23 +175,30 @@ abstract class EndpointCharacteristic extends string { // confidence = maximalConfidence() // } // } -// /** -// * Endpoints identified as "TaintedPathSink" by the standard JavaScript libraries are path injection sinks with maximal -// * confidence. -// */ -// private class TaintedPathSinkCharacteristic extends EndpointCharacteristic { -// TaintedPathSinkCharacteristic() { this = "TaintedPathSink" } -// override predicate appliesToEndpoint(DataFlow::Node n) { n instanceof TaintedPath::Sink } -// override predicate hasImplications( -// EndpointType endpointClass, boolean isPositiveIndicator, float confidence -// ) { -// endpointClass instanceof TaintedPathSinkType and -// isPositiveIndicator = true and -// confidence = maximalConfidence() -// } -// } /** - * Endpoints identified as "SqlInjectionSink" by the standard JavaScript libraries are SQL injection sinks with maximal + * Endpoints identified as "TaintedPathSink" by the standard Java libraries are path injection sinks with maximal + * confidence. + */ +private class TaintedPathSinkCharacteristic extends EndpointCharacteristic { + TaintedPathSinkCharacteristic() { this = "TaintedPathSink" } + + override predicate appliesToEndpoint(DataFlow::Node n) { + n.asExpr() = any(PathCreation p).getAnInput() + or + sinkNode(n, "create-file") + } + + override predicate hasImplications( + EndpointType endpointClass, boolean isPositiveIndicator, float confidence + ) { + endpointClass instanceof TaintedPathSinkType and + isPositiveIndicator = true and + confidence = maximalConfidence() + } +} + +/** + * Endpoints identified as "SqlInjectionSink" by the standard Java libraries are SQL injection sinks with maximal * confidence. */ private class SqlInjectionSinkCharacteristic extends EndpointCharacteristic { @@ -206,7 +216,7 @@ private class SqlInjectionSinkCharacteristic extends EndpointCharacteristic { } // /** -// * Endpoints identified as "NosqlInjectionSink" by the standard JavaScript libraries are NoSQL injection sinks with +// * Endpoints identified as "NosqlInjectionSink" by the standard Java libraries are NoSQL injection sinks with // * maximal confidence. // */ // private class NosqlInjectionSinkCharacteristic extends EndpointCharacteristic { diff --git a/java/ql/experimental/adaptivethreatmodeling/lib/experimental/adaptivethreatmodeling/TaintedPathATM.qll b/java/ql/experimental/adaptivethreatmodeling/lib/experimental/adaptivethreatmodeling/TaintedPathATM.qll index 55b295c72ba..8074aaa3403 100644 --- a/java/ql/experimental/adaptivethreatmodeling/lib/experimental/adaptivethreatmodeling/TaintedPathATM.qll +++ b/java/ql/experimental/adaptivethreatmodeling/lib/experimental/adaptivethreatmodeling/TaintedPathATM.qll @@ -3,16 +3,18 @@ * * A taint-tracking configuration for reasoning about path injection vulnerabilities. * Defines shared code used by the path injection boosted query. + * Largely copied from java/ql/src/Security/CWE/CWE-022/TaintedPath.ql. */ -import semmle.javascript.heuristics.SyntacticHeuristics -import semmle.javascript.security.dataflow.TaintedPathCustomizations +import java +import semmle.code.java.security.PathSanitizer import AdaptiveThreatModeling +import semmle.code.java.dataflow.FlowSources class TaintedPathAtmConfig extends AtmConfig { TaintedPathAtmConfig() { this = "TaintedPathAtmConfig" } - override predicate isKnownSource(DataFlow::Node source) { source instanceof TaintedPath::Source } + override predicate isKnownSource(DataFlow::Node source) { source instanceof RemoteFlowSource } override EndpointType getASinkEndpointType() { result instanceof TaintedPathSinkType } @@ -21,44 +23,60 @@ class TaintedPathAtmConfig extends AtmConfig { * query, except additional ATM sinks have been added to the `isSink` predicate. */ - override predicate isSink(DataFlow::Node sink, DataFlow::FlowLabel label) { - label = sink.(TaintedPath::Sink).getAFlowLabel() - or - // Allow effective sinks to have any taint label - isEffectiveSink(sink) + override predicate isSanitizer(DataFlow::Node sanitizer) { + sanitizer.getType() instanceof BoxedType or + sanitizer.getType() instanceof PrimitiveType or + sanitizer.getType() instanceof NumberType or + sanitizer instanceof PathInjectionSanitizer } - override predicate isSanitizer(DataFlow::Node node) { node instanceof TaintedPath::Sanitizer } - - override predicate isSanitizerGuard(TaintTracking::SanitizerGuardNode node) { - node instanceof BarrierGuardNodeAsSanitizerGuardNode + override predicate isAdditionalTaintStep(DataFlow::Node n1, DataFlow::Node n2) { + any(TaintedPathAdditionalTaintStep s).step(n1, n2) + } } - override predicate isAdditionalFlowStep( - DataFlow::Node src, DataFlow::Node dst, DataFlow::FlowLabel srclabel, - DataFlow::FlowLabel dstlabel - ) { - TaintedPath::isAdditionalTaintedPathFlowStep(src, dst, srclabel, dstlabel) - } -} +/* + * Models a very basic guard for the tainted path queries. + * TODO: Copied from java/ql/src/Security/CWE/CWE-022/TaintedPathCommon.qll because I couldn't figure out how to import it. + */ /** - * This class provides sanitizer guards for path injection. + * A unit class for adding additional taint steps. * - * The standard library path injection query uses a data flow configuration, and therefore defines - * barrier nodes. However we're using a taint tracking configuration for path injection to find new - * kinds of less certain results. Since taint tracking configurations use sanitizer guards instead - * of barrier guards, we port the barrier guards for the boosted query from the standard library to - * sanitizer guards here. + * Extend this class to add additional taint steps that should apply to tainted path flow configurations. */ -private class BarrierGuardNodeAsSanitizerGuardNode extends TaintTracking::LabeledSanitizerGuardNode { - BarrierGuardNodeAsSanitizerGuardNode() { this instanceof TaintedPath::BarrierGuardNode } +class TaintedPathAdditionalTaintStep extends Unit { + abstract predicate step(DataFlow::Node n1, DataFlow::Node n2); +} - override predicate sanitizes(boolean outcome, Expr e) { - blocks(outcome, e) or blocks(outcome, e, _) +private class DefaultTaintedPathAdditionalTaintStep extends TaintedPathAdditionalTaintStep { + override predicate step(DataFlow::Node n1, DataFlow::Node n2) { + exists(Argument a | + a = n1.asExpr() and + a.getCall() = n2.asExpr() and + a = any(TaintPreservingUriCtorParam tpp).getAnArgument() + ) + } } - override predicate sanitizes(boolean outcome, Expr e, DataFlow::FlowLabel label) { - sanitizes(outcome, e) and exists(label) +private class TaintPreservingUriCtorParam extends Parameter { + TaintPreservingUriCtorParam() { + exists(Constructor ctor, int idx, int nParams | + ctor.getDeclaringType() instanceof TypeUri and + this = ctor.getParameter(idx) and + nParams = ctor.getNumberOfParameters() + | + // URI(String scheme, String ssp, String fragment) + idx = 1 and nParams = 3 + or + // URI(String scheme, String host, String path, String fragment) + idx = [1, 2] and nParams = 4 + or + // URI(String scheme, String authority, String path, String query, String fragment) + idx = 2 and nParams = 5 + or + // URI(String scheme, String userInfo, String host, int port, String path, String query, String fragment) + idx = 4 and nParams = 7 + ) } } diff --git a/java/ql/experimental/adaptivethreatmodeling/src/ExtractPositiveExamples.ql b/java/ql/experimental/adaptivethreatmodeling/src/ExtractPositiveExamples.ql index 5557ad26ae2..609b2cbe4ab 100644 --- a/java/ql/experimental/adaptivethreatmodeling/src/ExtractPositiveExamples.ql +++ b/java/ql/experimental/adaptivethreatmodeling/src/ExtractPositiveExamples.ql @@ -12,6 +12,7 @@ import semmle.code.java.dataflow.TaintTracking private import experimental.adaptivethreatmodeling.EndpointCharacteristics as EndpointCharacteristics private import experimental.adaptivethreatmodeling.ATMConfig as AtmConfig private import experimental.adaptivethreatmodeling.SqlInjectionATM as SqlInjectionAtm +private import experimental.adaptivethreatmodeling.TaintedPathATM as TaintedPathAtm from DataFlow::Node sink, AtmConfig::AtmConfig config, @@ -20,4 +21,4 @@ where characteristic.appliesToEndpoint(sink) and confidence >= characteristic.maximalConfidence() and characteristic.hasImplications(config.getASinkEndpointType(), true, confidence) -select sink, "Sink of type " + characteristic + " with confidence " + confidence.toString() +select sink, characteristic.toString() diff --git a/java/ql/experimental/adaptivethreatmodeling/src/ExtractSinkCandidatesWithFlow.ql b/java/ql/experimental/adaptivethreatmodeling/src/ExtractSinkCandidatesWithFlow.ql index 219a7317d49..0beca8fe6e9 100644 --- a/java/ql/experimental/adaptivethreatmodeling/src/ExtractSinkCandidatesWithFlow.ql +++ b/java/ql/experimental/adaptivethreatmodeling/src/ExtractSinkCandidatesWithFlow.ql @@ -4,15 +4,10 @@ * * Note: This query does not actually classify the endpoints using the model. * - * TODO: Produce CSV/JSON/SARIF output describing these endpoints (probably just a URL for each endpoint). - * - * @name SQL database query built from user-controlled sources (experimental) - * @description Building a database query from user-controlled sources is vulnerable to insertion of - * malicious code by the user. + * @name Sink candidates with flow (experimental) + * @description Sink candidates with flow from a source * @kind problem - * @problem.severity error - * @security-severity 8.8 - * @id java/ml-powered/sql-injection + * @id java/ml-powered/sink-candidates-with-flow * @tags experimental security */ @@ -21,10 +16,19 @@ import semmle.code.java.dataflow.TaintTracking private import experimental.adaptivethreatmodeling.ATMConfig as AtmConfig // private import experimental.adaptivethreatmodeling.NosqlInjectionATM as NosqlInjectionAtm private import experimental.adaptivethreatmodeling.SqlInjectionATM as SqlInjectionAtm +private import experimental.adaptivethreatmodeling.TaintedPathATM as TaintedPathAtm -// private import experimental.adaptivethreatmodeling.TaintedPathATM as TaintedPathAtm // private import experimental.adaptivethreatmodeling.XssATM as XssAtm // private import experimental.adaptivethreatmodeling.XssThroughDomATM as XssThroughDomAtm -from DataFlow::PathNode sink -where exists(AtmConfig::AtmConfig queryConfig | queryConfig.isSinkCandidateWithFlow(sink)) -select sink.getNode(), "SQL injection sink candidate" +from DataFlow::PathNode sink, string message +where + // The message is the concatenation of all relevant configs + message = + concat(AtmConfig::AtmConfig config | + config.isSinkCandidateWithFlow(sink) + | + config.getASinkEndpointType().getDescription() + ", " + order by + config.getASinkEndpointType().getDescription() + ) +select sink.getNode(), message