Skip to content

Commit 9be9636

Browse files
MathiasVPMathiasVP
authored
Merge pull request #11670 from atorralba/atorralba/swift/predicate-injection
Swift: Add predicate injection query
2 parents 5b11708 + 30aa9b2 commit 9be9636

File tree

10 files changed

+200
-0
lines changed

10 files changed

+200
-0
lines changed

‎swift/ql/lib/codeql/swift/dataflow/ExternalFlow.qll

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ private module Frameworks {
9090
private import codeql.swift.frameworks.StandardLibrary.WebView
9191
private import codeql.swift.frameworks.Alamofire.Alamofire
9292
private import codeql.swift.security.PathInjection
93+
private import codeql.swift.security.PredicateInjection
9394
}
9495

9596
/**

‎swift/ql/lib/codeql/swift/security/PredicateInjection.qll

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
/** Provides classes and predicates to reason about predicate injection vulnerabilities. */
2+
3+
import swift
4+
private import codeql.swift.dataflow.DataFlow
5+
private import codeql.swift.dataflow.ExternalFlow
6+
7+
/** A data flow sink for predicate injection vulnerabilities. */
8+
abstract class PredicateInjectionSink extends DataFlow::Node { }
9+
10+
/** A sanitizer for predicate injection vulnerabilities. */
11+
abstract class PredicateInjectionSanitizer extends DataFlow::Node { }
12+
13+
/**
14+
* A unit class for adding additional taint steps.
15+
*
16+
* Extend this class to add additional taint steps that should apply to paths related to
17+
* predicate injection vulnerabilities.
18+
*/
19+
class PredicateInjectionAdditionalTaintStep extends Unit {
20+
/**
21+
* Holds if the step from `node1` to `node2` should be considered a taint
22+
* step for paths related to predicate injection vulnerabilities.
23+
*/
24+
abstract predicate step(DataFlow::Node n1, DataFlow::Node n2);
25+
}
26+
27+
private class DefaultPredicateInjectionSink extends PredicateInjectionSink {
28+
DefaultPredicateInjectionSink() { sinkNode(this, "predicate-injection") }
29+
}
30+
31+
private class PredicateInjectionSinkCsv extends SinkModelCsv {
32+
override predicate row(string row) {
33+
row =
34+
[
35+
";NSPredicate;true;init(format:argumentArray:);;;Argument[0];predicate-injection",
36+
";NSPredicate;true;init(format:arguments:);;;Argument[0];predicate-injection",
37+
";NSPredicate;true;init(format:_:);;;Argument[0];predicate-injection",
38+
";NSPredicate;true;init(fromMetadataQueryString:);;;Argument[0];predicate-injection"
39+
]
40+
}
41+
}

‎swift/ql/lib/codeql/swift/security/PredicateInjectionQuery.qll

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
/**
2+
* Provides a taint-tracking configuration for reasoning about predicate injection
3+
* vulnerabilities.
4+
*/
5+
6+
import swift
7+
private import codeql.swift.dataflow.DataFlow
8+
private import codeql.swift.dataflow.FlowSources
9+
private import codeql.swift.dataflow.TaintTracking
10+
private import codeql.swift.security.PredicateInjection
11+
12+
/**
13+
* A taint-tracking configuration for predicate injection vulnerabilities.
14+
*/
15+
class PredicateInjectionConf extends TaintTracking::Configuration {
16+
PredicateInjectionConf() { this = "PredicateInjectionConf" }
17+
18+
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
19+
20+
override predicate isSink(DataFlow::Node sink) { sink instanceof PredicateInjectionSink }
21+
22+
override predicate isSanitizer(DataFlow::Node sanitizer) {
23+
sanitizer instanceof PredicateInjectionSanitizer
24+
}
25+
26+
override predicate isAdditionalTaintStep(DataFlow::Node n1, DataFlow::Node n2) {
27+
any(PredicateInjectionAdditionalTaintStep s).step(n1, n2)
28+
}
29+
}

‎swift/ql/src/queries/Security/CWE-943/PredicateInjection.qhelp

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
2+
<qhelp>
3+
<overview>
4+
<p>
5+
Predicates represent logical conditions that can be used to check whether an object matches them.
6+
If a predicate is built from user-provided data without sufficient sanitization, an attacker may
7+
be able to change the overall meaning of the predicate.
8+
</p>
9+
</overview>
10+
<recommendation>
11+
<p>
12+
When building a predicate from untrusted data, you should either pass it to the appropriate <code>arguments</code> parameter during initialization, or as an array of substitution variables before evaluation. You should not append or concatenate it to the body of the predicate.
13+
</p>
14+
</recommendation>
15+
<example>
16+
<p>
17+
In the following insecure example, <code>NSPredicate</code> is built directly from data obtained from an HTTP request. This is untrusted, and can be arbitrarily set by an attacker to alter the meaning of the predicate:
18+
</p>
19+
<sample src="PredicateInjectionBad.swift" />
20+
<p>
21+
A better way to do this is to use the <code>arguments</code> parameter of <code>NSPredicate</code>'s constructor. This prevents attackers from altering the meaning of the predicate, even if they control the externally obtained data, as seen in the following secure example:
22+
</p>
23+
<sample src="PredicateInjectionGood.swift" />
24+
</example>
25+
<references>
26+
<li>Apple Developer Documentation: <a href="https://developer.apple.com/documentation/foundation/nspredicate">NSPredicate</a> </li>
27+
</references>
28+
</qhelp>

‎swift/ql/src/queries/Security/CWE-943/PredicateInjection.ql

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
/**
2+
* @name Predicate built from user-controlled sources
3+
* @description Building an NSPredicate from user-controlled sources may lead to attackers
4+
* changing the predicate's intended logic.
5+
* @kind path-problem
6+
* @problem.severity error
7+
* @security-severity 8.8
8+
* @precision high
9+
* @id swift/predicate-injection
10+
* @tags security
11+
* external/cwe/cwe-943
12+
*/
13+
14+
import swift
15+
import codeql.swift.dataflow.DataFlow
16+
import codeql.swift.security.PredicateInjectionQuery
17+
import DataFlow::PathGraph
18+
19+
from DataFlow::PathNode source, DataFlow::PathNode sink
20+
where any(PredicateInjectionConf c).hasFlowPath(source, sink)
21+
select sink.getNode(), source, sink, "This predicate depends on a $@.", source.getNode(),
22+
"user-provided value"

‎swift/ql/src/queries/Security/CWE-943/PredicateInjectionBad.swift

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
let remoteString = try String(contentsOf: URL(string: "https://example.com/")!)
2+
3+
let filenames: [String] = ["img1.png", "img2.png", "img3.png", "img.txt", "img.csv"]
4+
5+
let predicate = NSPredicate(format: "SELF LIKE \(remoteString)")
6+
let filtered = filenames.filter(){ filename in
7+
predicate.evaluate(with: filename)
8+
}
9+
print(filtered)

‎swift/ql/src/queries/Security/CWE-943/PredicateInjectionGood.swift

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
let remoteString = try String(contentsOf: URL(string: "https://example.com/")!)
2+
3+
let filenames: [String] = ["img1.png", "img2.png", "img3.png", "img.txt", "img.csv"]
4+
5+
let predicate = NSPredicate(format: "SELF LIKE %@", remoteString)
6+
let filtered = filenames.filter(){ filename in
7+
predicate.evaluate(with: filename)
8+
}
9+
print(filtered)

‎swift/ql/test/query-tests/Security/CWE-946/PredicateInjectionTest.expected

Whitespace-only changes.

‎swift/ql/test/query-tests/Security/CWE-946/PredicateInjectionTest.ql

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
import swift
2+
import codeql.swift.dataflow.DataFlow
3+
import codeql.swift.security.PredicateInjectionQuery
4+
import TestUtilities.InlineExpectationsTest
5+
6+
class PredicateInjectionTest extends InlineExpectationsTest {
7+
PredicateInjectionTest() { this = "PredicateInjectionTest" }
8+
9+
override string getARelevantTag() { result = "hasPredicateInjection" }
10+
11+
override predicate hasActualResult(Location location, string element, string tag, string value) {
12+
exists(
13+
PredicateInjectionConf config, DataFlow::Node source, DataFlow::Node sink, Expr sinkExpr
14+
|
15+
config.hasFlow(source, sink) and
16+
sinkExpr = sink.asExpr() and
17+
location = sinkExpr.getLocation() and
18+
element = sinkExpr.toString() and
19+
tag = "hasPredicateInjection" and
20+
value = source.asExpr().getLocation().getStartLine().toString()
21+
)
22+
}
23+
}

‎swift/ql/test/query-tests/Security/CWE-946/predicateInjection.swift

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
// --- stubs ---
2+
struct URL {
3+
init?(string: String) {}
4+
}
5+
6+
extension String {
7+
init(contentsOf: URL) {
8+
let data = ""
9+
self.init(data)
10+
}
11+
}
12+
13+
class NSPredicate {
14+
init(format: String, argumentArray: [Any]?) {}
15+
init(format: String, arguments: CVaListPointer) {}
16+
init(format: String, _: CVarArg...) {}
17+
init?(fromMetadataQueryString: String) {}
18+
}
19+
20+
// --- tests ---
21+
22+
func test() {
23+
let remoteString = String(contentsOf: URL(string: "http://example.com/")!)
24+
let safeString = "safe"
25+
26+
NSPredicate(format: remoteString, argumentArray: []) // $ hasPredicateInjection=23
27+
NSPredicate(format: safeString, argumentArray: []) // Safe
28+
NSPredicate(format: safeString, argumentArray: [remoteString]) // Safe
29+
NSPredicate(format: remoteString, arguments: CVaListPointer(_fromUnsafeMutablePointer: UnsafeMutablePointer(bitPattern: 0)!)) // $ hasPredicateInjection=23
30+
NSPredicate(format: safeString, arguments: CVaListPointer(_fromUnsafeMutablePointer: UnsafeMutablePointer(bitPattern: 0)!)) // Safe
31+
NSPredicate(format: remoteString) // $ hasPredicateInjection=23
32+
NSPredicate(format: safeString) // Safe
33+
NSPredicate(format: remoteString, "" as! CVarArg) // $ hasPredicateInjection=23
34+
NSPredicate(format: safeString, "" as! CVarArg) // Safe
35+
NSPredicate(format: safeString, remoteString as! CVarArg) // Safe
36+
NSPredicate(fromMetadataQueryString: remoteString) // $ hasPredicateInjection=23
37+
NSPredicate(fromMetadataQueryString: safeString) // Safe
38+
}

0 commit comments

Comments
 (0)