From 0ff607c930a10eddb4da76f6c7d0893e77ca4e19 Mon Sep 17 00:00:00 2001 From: Jeroen Ketema Date: Mon, 3 Apr 2023 16:21:07 +0200 Subject: [PATCH] Swift: Rewrite `XXEQuery` to use `DataFlow::ConfigSig` --- .../ql/lib/codeql/swift/security/XXEQuery.qll | 22 ++++++++++++++++++- swift/ql/src/queries/Security/CWE-611/XXE.ql | 6 ++--- .../query-tests/Security/CWE-611/XXETest.ql | 4 ++-- 3 files changed, 26 insertions(+), 6 deletions(-) diff --git a/swift/ql/lib/codeql/swift/security/XXEQuery.qll b/swift/ql/lib/codeql/swift/security/XXEQuery.qll index 2ffba705b3f..1d7bb4bce3d 100644 --- a/swift/ql/lib/codeql/swift/security/XXEQuery.qll +++ b/swift/ql/lib/codeql/swift/security/XXEQuery.qll @@ -12,7 +12,7 @@ import codeql.swift.security.XXEExtensions /** * A taint-tracking configuration for XML external entities (XXE) vulnerabilities. */ -class XxeConfiguration extends TaintTracking::Configuration { +deprecated class XxeConfiguration extends TaintTracking::Configuration { XxeConfiguration() { this = "XxeConfiguration" } override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource } @@ -25,3 +25,23 @@ class XxeConfiguration extends TaintTracking::Configuration { any(XxeAdditionalTaintStep s).step(n1, n2) } } + +/** + * A taint-tracking configuration for XML external entities (XXE) vulnerabilities. + */ +module XxeConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource } + + predicate isSink(DataFlow::Node sink) { sink instanceof XxeSink } + + predicate isBarrier(DataFlow::Node sanitizer) { sanitizer instanceof XxeSanitizer } + + predicate isAdditionalFlowStep(DataFlow::Node n1, DataFlow::Node n2) { + any(XxeAdditionalTaintStep s).step(n1, n2) + } +} + +/** + * Detect taint flow of XML external entities (XXE) vulnerabilities. + */ +module XxeFlow = TaintTracking::Global; diff --git a/swift/ql/src/queries/Security/CWE-611/XXE.ql b/swift/ql/src/queries/Security/CWE-611/XXE.ql index 04c27949c11..a09249e3e4f 100644 --- a/swift/ql/src/queries/Security/CWE-611/XXE.ql +++ b/swift/ql/src/queries/Security/CWE-611/XXE.ql @@ -16,10 +16,10 @@ import swift import codeql.swift.dataflow.DataFlow import codeql.swift.security.XXEQuery -import DataFlow::PathGraph +import XxeFlow::PathGraph -from DataFlow::PathNode source, DataFlow::PathNode sink -where any(XxeConfiguration c).hasFlowPath(source, sink) +from XxeFlow::PathNode source, XxeFlow::PathNode sink +where XxeFlow::flowPath(source, sink) select sink.getNode(), source, sink, "XML parsing depends on a $@ without guarding against external entity expansion.", source.getNode(), "user-provided value" diff --git a/swift/ql/test/query-tests/Security/CWE-611/XXETest.ql b/swift/ql/test/query-tests/Security/CWE-611/XXETest.ql index 757e45a102b..daa46b98c8d 100644 --- a/swift/ql/test/query-tests/Security/CWE-611/XXETest.ql +++ b/swift/ql/test/query-tests/Security/CWE-611/XXETest.ql @@ -15,8 +15,8 @@ class XxeTest extends InlineExpectationsTest { override string getARelevantTag() { result = "hasXXE" } override predicate hasActualResult(Location location, string element, string tag, string value) { - exists(XxeConfiguration config, DataFlow::Node source, DataFlow::Node sink, Expr sinkExpr | - config.hasFlow(source, sink) and + exists(DataFlow::Node source, DataFlow::Node sink, Expr sinkExpr | + XxeFlow::flow(source, sink) and sinkExpr = sink.asExpr() and location = sinkExpr.getLocation() and element = sinkExpr.toString() and