Skip to content

Commit 1deacf4

Browse files
geoffw0geoffw0
authored
Merge pull request github#13660 from geoffw0/regexinjection
Swift: Query for regular expression injection
2 parents 5d8b203 + 869ad2e commit 1deacf4

File tree

10 files changed

+454
-0
lines changed

10 files changed

+454
-0
lines changed

‎swift/ql/lib/codeql/swift/security/regex/RegexInjectionExtensions.qll

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
/**
2+
* Provides classes and predicates to reason about regular expression injection
3+
* vulnerabilities.
4+
*/
5+
6+
import swift
7+
import codeql.swift.dataflow.DataFlow
8+
import codeql.swift.dataflow.ExternalFlow
9+
import codeql.swift.regex.Regex
10+
11+
/**
12+
* A data flow sink for regular expression injection vulnerabilities.
13+
*/
14+
abstract class RegexInjectionSink extends DataFlow::Node { }
15+
16+
/**
17+
* A barrier for regular expression injection vulnerabilities.
18+
*/
19+
abstract class RegexInjectionBarrier extends DataFlow::Node { }
20+
21+
/**
22+
* A unit class for adding additional flow steps.
23+
*/
24+
class RegexInjectionAdditionalFlowStep extends Unit {
25+
/**
26+
* Holds if the step from `node1` to `node2` should be considered a flow
27+
* step for paths related to regular expression injection vulnerabilities.
28+
*/
29+
abstract predicate step(DataFlow::Node node1, DataFlow::Node node2);
30+
}
31+
32+
/**
33+
* A sink that is a regular expression evaluation defined in the Regex library.
34+
* This includes various methods that consume a regular expression string, but
35+
* in general misses cases where a regular expression string is converted into
36+
* an object (such as a `Regex` or `NSRegularExpression`) for later evaluation.
37+
* These cases are modeled separately.
38+
*/
39+
private class EvalRegexInjectionSink extends RegexInjectionSink {
40+
EvalRegexInjectionSink() { this.asExpr() = any(RegexEval e).getRegexInput() }
41+
}
42+
43+
/**
44+
* A sink that is a regular expression use defined in a CSV model.
45+
*/
46+
private class DefaultRegexInjectionSink extends RegexInjectionSink {
47+
DefaultRegexInjectionSink() { sinkNode(this, "regex-use") }
48+
}
49+
50+
private class RegexInjectionSinks extends SinkModelCsv {
51+
override predicate row(string row) {
52+
row =
53+
[
54+
";Regex;true;init(_:);;;Argument[0];regex-use",
55+
";Regex;true;init(_:as:);;;Argument[0];regex-use",
56+
";NSRegularExpression;true;init(pattern:options:);;;Argument[0];regex-use",
57+
]
58+
}
59+
}

‎swift/ql/lib/codeql/swift/security/regex/RegexInjectionQuery.qll

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
/**
2+
* Provides a taint-tracking configuration for detecting regular expression
3+
* injection vulnerabilities.
4+
*/
5+
6+
import swift
7+
import codeql.swift.dataflow.DataFlow
8+
import codeql.swift.dataflow.TaintTracking
9+
import codeql.swift.dataflow.FlowSources
10+
import codeql.swift.security.regex.RegexInjectionExtensions
11+
12+
/**
13+
* A taint configuration for detecting regular expression injection vulnerabilities.
14+
*/
15+
module RegexInjectionConfig implements DataFlow::ConfigSig {
16+
predicate isSource(DataFlow::Node source) { source instanceof FlowSource }
17+
18+
predicate isSink(DataFlow::Node sink) { sink instanceof RegexInjectionSink }
19+
20+
predicate isBarrier(DataFlow::Node barrier) { barrier instanceof RegexInjectionBarrier }
21+
22+
predicate isAdditionalFlowStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
23+
any(RegexInjectionAdditionalFlowStep s).step(nodeFrom, nodeTo)
24+
}
25+
}
26+
27+
/**
28+
* Detect taint flow of tainted data that reaches a regular expression sink.
29+
*/
30+
module RegexInjectionFlow = TaintTracking::Global<RegexInjectionConfig>;

‎swift/ql/src/change-notes/2023-07-04-regex-injection.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: newQuery
3+
---
4+
* Added new query "Regular expression injection" (`swift/regex-injection`). The query finds places where user input is used to construct a regular expression without proper escaping.

‎swift/ql/src/queries/Security/CWE-730/RegexInjection.qhelp

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
<overview>
7+
<p>
8+
Constructing a regular expression with unsanitized user input is dangerous,
9+
since a malicious user may be able to modify the meaning of the expression. They
10+
may be able to cause unexpected program behaviour, or perform a denial-of-service
11+
attack. For example, they may provide a regular expression fragment that takes
12+
exponential time to evaluate in the worst case.
13+
</p>
14+
</overview>
15+
16+
<recommendation>
17+
<p>
18+
Before embedding user input into a regular expression, use a sanitization
19+
function such as <code>NSRegularExpression::escapedPattern(for:)</code> to escape
20+
meta-characters that have special meaning.
21+
</p>
22+
</recommendation>
23+
24+
<example>
25+
<p>
26+
The following examples construct regular expressions from user input without
27+
sanitizing it first:
28+
</p>
29+
<sample src="RegexInjectionBad.swift" />
30+
<p>
31+
If user input is used to construct a regular expression it should be sanitized
32+
first. This ensures that the user cannot insert characters that have special
33+
meanings in regular expressions.
34+
</p>
35+
<sample src="RegexInjectionGood.swift" />
36+
</example>
37+
38+
<references>
39+
<li>
40+
OWASP:
41+
<a href="https://www.owasp.org/index.php/Regular_expression_Denial_of_Service_-_ReDoS">Regular expression Denial of Service - ReDoS</a>.
42+
</li>
43+
<li>
44+
Wikipedia: <a href="https://en.wikipedia.org/wiki/ReDoS">ReDoS</a>.
45+
</li>
46+
<li>
47+
Swift: <a href="https://developer.apple.com/documentation/foundation/nsregularexpression/1408386-escapedpattern">NSRegularExpression.escapedPattern(for:)</a>.
48+
</li>
49+
</references>
50+
</qhelp>

‎swift/ql/src/queries/Security/CWE-730/RegexInjection.ql

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
/**
2+
* @name Regular expression injection
3+
* @description User input should not be used in regular expressions without first being escaped,
4+
* otherwise a malicious user may be able to provide a regex that could require
5+
* exponential time on certain inputs.
6+
* @kind path-problem
7+
* @problem.severity error
8+
* @security-severity 7.5
9+
* @precision high
10+
* @id swift/regex-injection
11+
* @tags security
12+
* external/cwe/cwe-730
13+
* external/cwe/cwe-400
14+
*/
15+
16+
import swift
17+
import codeql.swift.dataflow.DataFlow
18+
import codeql.swift.security.regex.RegexInjectionQuery
19+
import RegexInjectionFlow::PathGraph
20+
21+
from RegexInjectionFlow::PathNode sourceNode, RegexInjectionFlow::PathNode sinkNode
22+
where RegexInjectionFlow::flowPath(sourceNode, sinkNode)
23+
select sinkNode.getNode(), sourceNode, sinkNode,
24+
"This regular expression is constructed from a $@.", sourceNode.getNode(), "user-provided value"

‎swift/ql/src/queries/Security/CWE-730/RegexInjectionBad.swift

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
func processRemoteInput(remoteInput: String) {
2+
...
3+
4+
// BAD: Unsanitized user input is used to construct a regular expression
5+
let regex1 = try Regex(remoteInput)
6+
7+
// BAD: Unsanitized user input is used to construct a regular expression
8+
let regexStr = "abc|\(remoteInput)"
9+
let regex2 = try NSRegularExpression(pattern: regexStr)
10+
11+
...
12+
}

‎swift/ql/src/queries/Security/CWE-730/RegexInjectionGood.swift

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
func processRemoteInput(remoteInput: String) {
2+
...
3+
4+
// GOOD: Regular expression is not derived from user input
5+
let regex1 = try Regex(myRegex)
6+
7+
// GOOD: User input is sanitized before being used to construct a regular expression
8+
let escapedInput = NSRegularExpression.escapedPattern(for: remoteInput)
9+
let regexStr = "abc|\(escapedInput)"
10+
let regex2 = try NSRegularExpression(pattern: regexStr)
11+
12+
...
13+
}

‎swift/ql/test/query-tests/Security/CWE-730/RegexInjection.expected

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
edges
2+
| tests.swift:95:22:95:46 | call to String.init(contentsOf:) | tests.swift:101:16:101:16 | taintedString |
3+
| tests.swift:95:22:95:46 | call to String.init(contentsOf:) | tests.swift:104:16:104:40 | ... .+(_:_:) ... |
4+
| tests.swift:95:22:95:46 | call to String.init(contentsOf:) | tests.swift:106:16:106:16 | "..." |
5+
| tests.swift:95:22:95:46 | call to String.init(contentsOf:) | tests.swift:109:16:109:39 | ... ? ... : ... |
6+
| tests.swift:95:22:95:46 | call to String.init(contentsOf:) | tests.swift:110:16:110:37 | ... ? ... : ... |
7+
| tests.swift:95:22:95:46 | call to String.init(contentsOf:) | tests.swift:113:24:113:24 | taintedString |
8+
| tests.swift:95:22:95:46 | call to String.init(contentsOf:) | tests.swift:114:45:114:45 | taintedString |
9+
| tests.swift:95:22:95:46 | call to String.init(contentsOf:) | tests.swift:120:19:120:19 | taintedString |
10+
| tests.swift:95:22:95:46 | call to String.init(contentsOf:) | tests.swift:131:39:131:39 | taintedString |
11+
| tests.swift:95:22:95:46 | call to String.init(contentsOf:) | tests.swift:144:16:144:16 | remoteInput |
12+
| tests.swift:95:22:95:46 | call to String.init(contentsOf:) | tests.swift:147:39:147:39 | regexStr |
13+
| tests.swift:95:22:95:46 | call to String.init(contentsOf:) | tests.swift:162:17:162:17 | taintedString |
14+
| tests.swift:95:22:95:46 | call to String.init(contentsOf:) | tests.swift:164:17:164:17 | taintedString |
15+
| tests.swift:95:22:95:46 | call to String.init(contentsOf:) | tests.swift:167:17:167:17 | taintedString |
16+
| tests.swift:95:22:95:46 | call to String.init(contentsOf:) | tests.swift:170:17:170:17 | taintedString |
17+
| tests.swift:95:22:95:46 | call to String.init(contentsOf:) | tests.swift:173:17:173:17 | taintedString |
18+
| tests.swift:95:22:95:46 | call to String.init(contentsOf:) | tests.swift:176:17:176:17 | taintedString |
19+
| tests.swift:95:22:95:46 | call to String.init(contentsOf:) | tests.swift:179:17:179:17 | taintedString |
20+
| tests.swift:95:22:95:46 | call to String.init(contentsOf:) | tests.swift:182:17:182:17 | taintedString |
21+
| tests.swift:95:22:95:46 | call to String.init(contentsOf:) | tests.swift:185:17:185:17 | taintedString |
22+
| tests.swift:95:22:95:46 | call to String.init(contentsOf:) | tests.swift:190:21:190:21 | taintedString |
23+
nodes
24+
| tests.swift:95:22:95:46 | call to String.init(contentsOf:) | semmle.label | call to String.init(contentsOf:) |
25+
| tests.swift:101:16:101:16 | taintedString | semmle.label | taintedString |
26+
| tests.swift:104:16:104:40 | ... .+(_:_:) ... | semmle.label | ... .+(_:_:) ... |
27+
| tests.swift:106:16:106:16 | "..." | semmle.label | "..." |
28+
| tests.swift:109:16:109:39 | ... ? ... : ... | semmle.label | ... ? ... : ... |
29+
| tests.swift:110:16:110:37 | ... ? ... : ... | semmle.label | ... ? ... : ... |
30+
| tests.swift:113:24:113:24 | taintedString | semmle.label | taintedString |
31+
| tests.swift:114:45:114:45 | taintedString | semmle.label | taintedString |
32+
| tests.swift:120:19:120:19 | taintedString | semmle.label | taintedString |
33+
| tests.swift:131:39:131:39 | taintedString | semmle.label | taintedString |
34+
| tests.swift:144:16:144:16 | remoteInput | semmle.label | remoteInput |
35+
| tests.swift:147:39:147:39 | regexStr | semmle.label | regexStr |
36+
| tests.swift:162:17:162:17 | taintedString | semmle.label | taintedString |
37+
| tests.swift:164:17:164:17 | taintedString | semmle.label | taintedString |
38+
| tests.swift:167:17:167:17 | taintedString | semmle.label | taintedString |
39+
| tests.swift:170:17:170:17 | taintedString | semmle.label | taintedString |
40+
| tests.swift:173:17:173:17 | taintedString | semmle.label | taintedString |
41+
| tests.swift:176:17:176:17 | taintedString | semmle.label | taintedString |
42+
| tests.swift:179:17:179:17 | taintedString | semmle.label | taintedString |
43+
| tests.swift:182:17:182:17 | taintedString | semmle.label | taintedString |
44+
| tests.swift:185:17:185:17 | taintedString | semmle.label | taintedString |
45+
| tests.swift:190:21:190:21 | taintedString | semmle.label | taintedString |
46+
subpaths
47+
#select
48+
| tests.swift:101:16:101:16 | taintedString | tests.swift:95:22:95:46 | call to String.init(contentsOf:) | tests.swift:101:16:101:16 | taintedString | This regular expression is constructed from a $@. | tests.swift:95:22:95:46 | call to String.init(contentsOf:) | user-provided value |
49+
| tests.swift:104:16:104:40 | ... .+(_:_:) ... | tests.swift:95:22:95:46 | call to String.init(contentsOf:) | tests.swift:104:16:104:40 | ... .+(_:_:) ... | This regular expression is constructed from a $@. | tests.swift:95:22:95:46 | call to String.init(contentsOf:) | user-provided value |
50+
| tests.swift:106:16:106:16 | "..." | tests.swift:95:22:95:46 | call to String.init(contentsOf:) | tests.swift:106:16:106:16 | "..." | This regular expression is constructed from a $@. | tests.swift:95:22:95:46 | call to String.init(contentsOf:) | user-provided value |
51+
| tests.swift:109:16:109:39 | ... ? ... : ... | tests.swift:95:22:95:46 | call to String.init(contentsOf:) | tests.swift:109:16:109:39 | ... ? ... : ... | This regular expression is constructed from a $@. | tests.swift:95:22:95:46 | call to String.init(contentsOf:) | user-provided value |
52+
| tests.swift:110:16:110:37 | ... ? ... : ... | tests.swift:95:22:95:46 | call to String.init(contentsOf:) | tests.swift:110:16:110:37 | ... ? ... : ... | This regular expression is constructed from a $@. | tests.swift:95:22:95:46 | call to String.init(contentsOf:) | user-provided value |
53+
| tests.swift:113:24:113:24 | taintedString | tests.swift:95:22:95:46 | call to String.init(contentsOf:) | tests.swift:113:24:113:24 | taintedString | This regular expression is constructed from a $@. | tests.swift:95:22:95:46 | call to String.init(contentsOf:) | user-provided value |
54+
| tests.swift:114:45:114:45 | taintedString | tests.swift:95:22:95:46 | call to String.init(contentsOf:) | tests.swift:114:45:114:45 | taintedString | This regular expression is constructed from a $@. | tests.swift:95:22:95:46 | call to String.init(contentsOf:) | user-provided value |
55+
| tests.swift:120:19:120:19 | taintedString | tests.swift:95:22:95:46 | call to String.init(contentsOf:) | tests.swift:120:19:120:19 | taintedString | This regular expression is constructed from a $@. | tests.swift:95:22:95:46 | call to String.init(contentsOf:) | user-provided value |
56+
| tests.swift:131:39:131:39 | taintedString | tests.swift:95:22:95:46 | call to String.init(contentsOf:) | tests.swift:131:39:131:39 | taintedString | This regular expression is constructed from a $@. | tests.swift:95:22:95:46 | call to String.init(contentsOf:) | user-provided value |
57+
| tests.swift:144:16:144:16 | remoteInput | tests.swift:95:22:95:46 | call to String.init(contentsOf:) | tests.swift:144:16:144:16 | remoteInput | This regular expression is constructed from a $@. | tests.swift:95:22:95:46 | call to String.init(contentsOf:) | user-provided value |
58+
| tests.swift:147:39:147:39 | regexStr | tests.swift:95:22:95:46 | call to String.init(contentsOf:) | tests.swift:147:39:147:39 | regexStr | This regular expression is constructed from a $@. | tests.swift:95:22:95:46 | call to String.init(contentsOf:) | user-provided value |
59+
| tests.swift:162:17:162:17 | taintedString | tests.swift:95:22:95:46 | call to String.init(contentsOf:) | tests.swift:162:17:162:17 | taintedString | This regular expression is constructed from a $@. | tests.swift:95:22:95:46 | call to String.init(contentsOf:) | user-provided value |
60+
| tests.swift:164:17:164:17 | taintedString | tests.swift:95:22:95:46 | call to String.init(contentsOf:) | tests.swift:164:17:164:17 | taintedString | This regular expression is constructed from a $@. | tests.swift:95:22:95:46 | call to String.init(contentsOf:) | user-provided value |
61+
| tests.swift:167:17:167:17 | taintedString | tests.swift:95:22:95:46 | call to String.init(contentsOf:) | tests.swift:167:17:167:17 | taintedString | This regular expression is constructed from a $@. | tests.swift:95:22:95:46 | call to String.init(contentsOf:) | user-provided value |
62+
| tests.swift:170:17:170:17 | taintedString | tests.swift:95:22:95:46 | call to String.init(contentsOf:) | tests.swift:170:17:170:17 | taintedString | This regular expression is constructed from a $@. | tests.swift:95:22:95:46 | call to String.init(contentsOf:) | user-provided value |
63+
| tests.swift:173:17:173:17 | taintedString | tests.swift:95:22:95:46 | call to String.init(contentsOf:) | tests.swift:173:17:173:17 | taintedString | This regular expression is constructed from a $@. | tests.swift:95:22:95:46 | call to String.init(contentsOf:) | user-provided value |
64+
| tests.swift:176:17:176:17 | taintedString | tests.swift:95:22:95:46 | call to String.init(contentsOf:) | tests.swift:176:17:176:17 | taintedString | This regular expression is constructed from a $@. | tests.swift:95:22:95:46 | call to String.init(contentsOf:) | user-provided value |
65+
| tests.swift:179:17:179:17 | taintedString | tests.swift:95:22:95:46 | call to String.init(contentsOf:) | tests.swift:179:17:179:17 | taintedString | This regular expression is constructed from a $@. | tests.swift:95:22:95:46 | call to String.init(contentsOf:) | user-provided value |
66+
| tests.swift:182:17:182:17 | taintedString | tests.swift:95:22:95:46 | call to String.init(contentsOf:) | tests.swift:182:17:182:17 | taintedString | This regular expression is constructed from a $@. | tests.swift:95:22:95:46 | call to String.init(contentsOf:) | user-provided value |
67+
| tests.swift:185:17:185:17 | taintedString | tests.swift:95:22:95:46 | call to String.init(contentsOf:) | tests.swift:185:17:185:17 | taintedString | This regular expression is constructed from a $@. | tests.swift:95:22:95:46 | call to String.init(contentsOf:) | user-provided value |
68+
| tests.swift:190:21:190:21 | taintedString | tests.swift:95:22:95:46 | call to String.init(contentsOf:) | tests.swift:190:21:190:21 | taintedString | This regular expression is constructed from a $@. | tests.swift:95:22:95:46 | call to String.init(contentsOf:) | user-provided value |

‎swift/ql/test/query-tests/Security/CWE-730/RegexInjection.qlref

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
queries/Security/CWE-730/RegexInjection.ql

0 commit comments

Comments
 (0)