Skip to content

Commit e3e68b7

Browse files
authored
Merge pull request github#12642 from geoffw0/modernstring
Swift: Modernize the swift/string-length-conflation query
2 parents 6dd45b3 + c158f83 commit e3e68b7

File tree

7 files changed

+265
-178
lines changed

7 files changed

+265
-178
lines changed

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@
1616
* 1. The `namespace` column selects a package.
1717
* 2. The `type` column selects a type within that package.
1818
* 3. The `subtypes` is a boolean that indicates whether to jump to an
19-
* arbitrary subtype of that type.
19+
* arbitrary subtype of that type. Set this to `false` if leaving the `type`
20+
* blank (for example, a free function).
2021
* 4. The `name` column optionally selects a specific named member of the type.
2122
* 5. The `signature` column optionally restricts the named member. If
2223
* `signature` is blank then no such filtering is done. The format of the
Lines changed: 189 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,189 @@
1+
/**
2+
* Provides classes and predicates for reasoning about string length
3+
* conflation vulnerabilities.
4+
*/
5+
6+
import swift
7+
import codeql.swift.dataflow.DataFlow
8+
import codeql.swift.dataflow.ExternalFlow
9+
10+
/**
11+
* A type of Swift string encoding. This class is used as a flow state for
12+
* the string length conflation taint tracking configuration.
13+
*/
14+
class StringType extends string {
15+
string singular;
16+
string equivClass;
17+
string csvLabel;
18+
19+
StringType() {
20+
this = "String" and
21+
singular = "a String" and
22+
equivClass = "String" and
23+
csvLabel = "string-length"
24+
or
25+
this = "NSString" and
26+
singular = "an NSString" and
27+
equivClass = "NSString" and
28+
csvLabel = "nsstring-length"
29+
or
30+
this = "String.utf8" and
31+
singular = "a String.utf8" and
32+
equivClass = "String.utf8" and
33+
csvLabel = "string-utf8-length"
34+
or
35+
this = "String.utf16" and
36+
singular = "a String.utf16" and
37+
equivClass = "NSString" and
38+
csvLabel = "string-utf16-length"
39+
or
40+
this = "String.unicodeScalars" and
41+
singular = "a String.unicodeScalars" and
42+
equivClass = "String.unicodeScalars" and
43+
csvLabel = "string-unicodescalars-length"
44+
}
45+
46+
/**
47+
* Gets the equivalence class for this string type. If these are equal,
48+
* they should be treated as equivalent.
49+
*/
50+
string getEquivClass() { result = equivClass }
51+
52+
/**
53+
* Gets text for the singular form of this string type.
54+
*/
55+
string getSingular() { result = singular }
56+
57+
/**
58+
* Gets the label for this string type in CSV models.
59+
*/
60+
string getCsvLabel() { result = csvLabel }
61+
}
62+
63+
/**
64+
* A dataflow source for string length conflation vulnerabilities. That is,
65+
* a `DataFlow::Node` where a string length is generated.
66+
*/
67+
abstract class StringLengthConflationSource extends DataFlow::Node {
68+
/**
69+
* Gets the `StringType` for this source.
70+
*/
71+
abstract StringType getStringType();
72+
}
73+
74+
/**
75+
* A dataflow sink for string length conflation vulnerabilities. That is,
76+
* a `DataFlow::Node` where a string length is used.
77+
*/
78+
abstract class StringLengthConflationSink extends DataFlow::Node {
79+
/**
80+
* Gets the correct `StringType` for this sink.
81+
*/
82+
abstract StringType getCorrectStringType();
83+
}
84+
85+
/**
86+
* A sanitizer for string length conflation vulnerabilities.
87+
*/
88+
abstract class StringLengthConflationSanitizer extends DataFlow::Node { }
89+
90+
/**
91+
* A unit class for adding additional taint steps.
92+
*/
93+
class StringLengthConflationAdditionalTaintStep extends Unit {
94+
/**
95+
* Holds if the step from `node1` to `node2` should be considered a taint
96+
* step for paths related to string length conflation vulnerabilities.
97+
*/
98+
abstract predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo);
99+
}
100+
101+
/**
102+
* A source defined in a CSV model.
103+
*/
104+
private class DefaultStringLengthConflationSource extends StringLengthConflationSource {
105+
StringType stringType;
106+
107+
DefaultStringLengthConflationSource() { sourceNode(this, stringType.getCsvLabel()) }
108+
109+
override StringType getStringType() { result = stringType }
110+
}
111+
112+
private class StringLengthConflationSources extends SourceModelCsv {
113+
override predicate row(string row) {
114+
row =
115+
[
116+
";String;true;count;;;;string-length", ";NSString;true;length;;;;nsstring-length",
117+
";NSMutableString;true;length;;;;nsstring-length",
118+
]
119+
}
120+
}
121+
122+
/**
123+
* An extra source that don't currently fit into the CSV scheme.
124+
*/
125+
private class ExtraStringLengthConflationSource extends StringLengthConflationSource {
126+
StringType stringType;
127+
128+
ExtraStringLengthConflationSource() {
129+
exists(MemberRefExpr memberRef, string typeName |
130+
(
131+
// result of a call to `String.utf8.count`
132+
typeName = "String.UTF8View" and
133+
stringType = "String.utf8"
134+
or
135+
// result of a call to `String.utf16.count`
136+
typeName = "String.UTF16View" and
137+
stringType = "String.utf16"
138+
or
139+
// result of a call to `String.unicodeScalars.count`
140+
typeName = "String.UnicodeScalarView" and
141+
stringType = "String.unicodeScalars"
142+
) and
143+
memberRef.getBase().getType().(NominalType).getName() = typeName and
144+
memberRef.getMember().(VarDecl).getName() = "count" and
145+
this.asExpr() = memberRef
146+
)
147+
}
148+
149+
override StringType getStringType() { result = stringType }
150+
}
151+
152+
/**
153+
* A sink defined in a CSV model.
154+
*/
155+
private class DefaultStringLengthConflationSink extends StringLengthConflationSink {
156+
StringType stringType;
157+
158+
DefaultStringLengthConflationSink() { sinkNode(this, stringType.getCsvLabel()) }
159+
160+
override StringType getCorrectStringType() { result = stringType }
161+
}
162+
163+
private class StringLengthConflationSinks extends SinkModelCsv {
164+
override predicate row(string row) {
165+
row =
166+
[
167+
";Sequence;true;dropFirst(_:);;;Argument[0];string-length",
168+
";Sequence;true;dropLast(_:);;;Argument[0];string-length",
169+
";Sequence;true;prefix(_:);;;Argument[0];string-length",
170+
";Sequence;true;suffix(_:);;;Argument[0];string-length",
171+
";Collection;true;formIndex(_:offsetBy:);;;Argument[0..1];string-length",
172+
";Collection;true;formIndex(_:offsetBy:limitBy:);;;Argument[0..1];string-length",
173+
";Collection;true;removeFirst(_:);;;Argument[0];string-length",
174+
";RangeReplaceableCollection;true;removeLast(_:);;;Argument[0];string-length",
175+
";String;true;index(_:offsetBy:);;;Argument[0..1];string-length",
176+
";String;true;index(_:offsetBy:limitBy:);;;Argument[0..1];string-length",
177+
";String.Index;true;init(encodedOffset:);;;Argument[0];string-length",
178+
";NSRange;true;init(location:length:);;;Argument[0..1];nsstring-length",
179+
";NSString;true;character(at:);;;Argument[0];nsstring-length",
180+
";NSString;true;substring(from:);;;Argument[0];nsstring-length",
181+
";NSString;true;substring(to:);;;Argument[0];nsstring-length",
182+
";NSMutableString;true;character(at:);;;Argument[0];nsstring-length",
183+
";NSMutableString;true;substring(from:);;;Argument[0];nsstring-length",
184+
";NSMutableString;true;substring(to:);;;Argument[0];nsstring-length",
185+
";NSMutableString;true;insert(_:at:);;;Argument[1];nsstring-length",
186+
";;false;NSMakeRange(_:_:);;;Argument[0..1];nsstring-length",
187+
]
188+
}
189+
}
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
/**
2+
* Provides a taint-tracking configuration for reasoning about string length
3+
* conflation vulnerabilities.
4+
*/
5+
6+
import swift
7+
import codeql.swift.dataflow.DataFlow
8+
import codeql.swift.dataflow.TaintTracking
9+
import codeql.swift.security.StringLengthConflationExtensions
10+
11+
/**
12+
* A configuration for tracking string lengths originating from source that is
13+
* a `String` or an `NSString` object, to a sink of a different kind that
14+
* expects an incompatible measure of length.
15+
*/
16+
class StringLengthConflationConfiguration extends TaintTracking::Configuration {
17+
StringLengthConflationConfiguration() { this = "StringLengthConflationConfiguration" }
18+
19+
override predicate isSource(DataFlow::Node node, string flowstate) {
20+
flowstate = node.(StringLengthConflationSource).getStringType()
21+
}
22+
23+
override predicate isSink(DataFlow::Node node, string flowstate) {
24+
// Permit any *incorrect* flowstate, as those are the results the query
25+
// should report.
26+
exists(string correctFlowState |
27+
correctFlowState = node.(StringLengthConflationSink).getCorrectStringType() and
28+
flowstate.(StringType).getEquivClass() != correctFlowState.(StringType).getEquivClass()
29+
)
30+
}
31+
32+
override predicate isSanitizer(DataFlow::Node sanitizer) {
33+
sanitizer instanceof StringLengthConflationSanitizer
34+
}
35+
36+
override predicate isAdditionalTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
37+
any(StringLengthConflationAdditionalTaintStep s).step(nodeFrom, nodeTo)
38+
}
39+
}

0 commit comments

Comments
 (0)