Skip to content

Commit 4c0d02a

Browse files
geoffw0geoffw0
committed
Swift: Standardize the sources, sinks etc.
1 parent dfcad7f commit 4c0d02a

File tree

3 files changed

+176
-122
lines changed

3 files changed

+176
-122
lines changed

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

Lines changed: 161 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,14 @@ import swift
77
import codeql.swift.dataflow.DataFlow
88

99
/**
10-
* A flow state for encoding types of Swift string encoding.
10+
* A type of Swift string encoding. This class is used as a flow state for
11+
* the string length conflation taint tracking configuration.
1112
*/
12-
class StringLengthConflationFlowState extends string {
13-
string equivClass;
13+
class StringType extends string {
1414
string singular;
15+
string equivClass;
1516

16-
StringLengthConflationFlowState() {
17+
StringType() {
1718
this = "String" and singular = "a String" and equivClass = "String"
1819
or
1920
this = "NSString" and singular = "an NSString" and equivClass = "NSString"
@@ -38,3 +39,159 @@ class StringLengthConflationFlowState extends string {
3839
*/
3940
string getSingular() { result = singular }
4041
}
42+
43+
/**
44+
* A dataflow source for string length conflation vulnerabilities. That is,
45+
* a `DataFlow::Node` where a string length is generated.
46+
*/
47+
abstract class StringLengthConflationSource extends DataFlow::Node {
48+
/**
49+
* Gets the `StringType` for this source.
50+
*/
51+
abstract StringType getStringType();
52+
}
53+
54+
/**
55+
* A dataflow sink for string length conflation vulnerabilities. That is,
56+
* a `DataFlow::Node` where a string length is used.
57+
*/
58+
abstract class StringLengthConflationSink extends DataFlow::Node {
59+
/**
60+
* Gets the correct `StringType` for this sink.
61+
*/
62+
abstract StringType getCorrectStringType();
63+
}
64+
65+
abstract class StringLengthConflationSanitizer extends DataFlow::Node { }
66+
67+
/**
68+
* A unit class for adding additional taint steps.
69+
*/
70+
class StringLengthConflationAdditionalTaintStep extends Unit {
71+
/**
72+
* Holds if the step from `node1` to `node2` should be considered a taint
73+
* step for paths related to string length conflation vulnerabilities.
74+
*/
75+
abstract predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo);
76+
}
77+
78+
private class DefaultStringLengthConflationSource extends StringLengthConflationSource {
79+
StringType stringType;
80+
81+
DefaultStringLengthConflationSource() {
82+
exists(MemberRefExpr memberRef, string className, string varName |
83+
memberRef.getBase().getType().(NominalType).getABaseType*().getName() = className and
84+
memberRef.getMember().(VarDecl).getName() = varName and
85+
this.asExpr() = memberRef and
86+
(
87+
// result of a call to `String.count`
88+
className = "String" and
89+
varName = "count" and
90+
stringType = "String"
91+
or
92+
// result of a call to `NSString.length`
93+
className = ["NSString", "NSMutableString"] and
94+
varName = "length" and
95+
stringType = "NSString"
96+
or
97+
// result of a call to `String.utf8.count`
98+
className = "String.UTF8View" and
99+
varName = "count" and
100+
stringType = "String.utf8"
101+
or
102+
// result of a call to `String.utf16.count`
103+
className = "String.UTF16View" and
104+
varName = "count" and
105+
stringType = "String.utf16"
106+
or
107+
// result of a call to `String.unicodeScalars.count`
108+
className = "String.UnicodeScalarView" and
109+
varName = "count" and
110+
stringType = "String.unicodeScalars"
111+
)
112+
)
113+
}
114+
115+
override StringType getStringType() { result = stringType }
116+
}
117+
118+
private class DefaultStringLengthConflationSink extends StringLengthConflationSink {
119+
StringType correctStringType;
120+
121+
DefaultStringLengthConflationSink() {
122+
exists(AbstractFunctionDecl funcDecl, CallExpr call, string funcName, int arg |
123+
(
124+
// arguments to method calls...
125+
exists(string className, ClassOrStructDecl c |
126+
(
127+
// `NSRange.init`
128+
className = "NSRange" and
129+
funcName = "init(location:length:)" and
130+
arg = [0, 1]
131+
or
132+
// `NSString.character`
133+
className = ["NSString", "NSMutableString"] and
134+
funcName = "character(at:)" and
135+
arg = 0
136+
or
137+
// `NSString.character`
138+
className = ["NSString", "NSMutableString"] and
139+
funcName = "substring(from:)" and
140+
arg = 0
141+
or
142+
// `NSString.character`
143+
className = ["NSString", "NSMutableString"] and
144+
funcName = "substring(to:)" and
145+
arg = 0
146+
or
147+
// `NSMutableString.insert`
148+
className = "NSMutableString" and
149+
funcName = "insert(_:at:)" and
150+
arg = 1
151+
) and
152+
c.getName() = className and
153+
c.getABaseTypeDecl*().(ClassOrStructDecl).getAMember() = funcDecl and
154+
call.getStaticTarget() = funcDecl and
155+
correctStringType = "NSString"
156+
)
157+
or
158+
// arguments to function calls...
159+
// `NSMakeRange`
160+
funcName = "NSMakeRange(_:_:)" and
161+
arg = [0, 1] and
162+
call.getStaticTarget() = funcDecl and
163+
correctStringType = "NSString"
164+
or
165+
// arguments to method calls...
166+
(
167+
// `String.dropFirst`, `String.dropLast`, `String.removeFirst`, `String.removeLast`
168+
funcName = ["dropFirst(_:)", "dropLast(_:)", "removeFirst(_:)", "removeLast(_:)"] and
169+
arg = 0
170+
or
171+
// `String.prefix`, `String.suffix`
172+
funcName = ["prefix(_:)", "suffix(_:)"] and
173+
arg = 0
174+
or
175+
// `String.Index.init`
176+
funcName = "init(encodedOffset:)" and
177+
arg = 0
178+
or
179+
// `String.index`
180+
funcName = ["index(_:offsetBy:)", "index(_:offsetBy:limitBy:)"] and
181+
arg = [0, 1]
182+
or
183+
// `String.formIndex`
184+
funcName = ["formIndex(_:offsetBy:)", "formIndex(_:offsetBy:limitBy:)"] and
185+
arg = [0, 1]
186+
) and
187+
call.getStaticTarget() = funcDecl and
188+
correctStringType = "String"
189+
) and
190+
// match up `funcName`, `arg`, `node`.
191+
funcDecl.getName() = funcName and
192+
call.getArgument(arg).getExpr() = this.asExpr()
193+
)
194+
}
195+
196+
override StringType getCorrectStringType() { result = correctStringType }
197+
}

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

Lines changed: 11 additions & 113 deletions
Original file line numberDiff line numberDiff line change
@@ -17,125 +17,23 @@ class StringLengthConflationConfiguration extends TaintTracking::Configuration {
1717
StringLengthConflationConfiguration() { this = "StringLengthConflationConfiguration" }
1818

1919
override predicate isSource(DataFlow::Node node, string flowstate) {
20-
exists(MemberRefExpr memberRef, string className, string varName |
21-
memberRef.getBase().getType().(NominalType).getABaseType*().getName() = className and
22-
memberRef.getMember().(VarDecl).getName() = varName and
23-
node.asExpr() = memberRef and
24-
(
25-
// result of a call to `String.count`
26-
className = "String" and
27-
varName = "count" and
28-
flowstate = "String"
29-
or
30-
// result of a call to `NSString.length`
31-
className = ["NSString", "NSMutableString"] and
32-
varName = "length" and
33-
flowstate = "NSString"
34-
or
35-
// result of a call to `String.utf8.count`
36-
className = "String.UTF8View" and
37-
varName = "count" and
38-
flowstate = "String.utf8"
39-
or
40-
// result of a call to `String.utf16.count`
41-
className = "String.UTF16View" and
42-
varName = "count" and
43-
flowstate = "String.utf16"
44-
or
45-
// result of a call to `String.unicodeScalars.count`
46-
className = "String.UnicodeScalarView" and
47-
varName = "count" and
48-
flowstate = "String.unicodeScalars"
49-
)
50-
)
51-
}
52-
53-
/**
54-
* Holds if `node` is a sink and `flowstate` is the *correct* flow state for
55-
* that sink. We actually want to report incorrect flow states.
56-
*/
57-
predicate isSinkImpl(DataFlow::Node node, string flowstate) {
58-
exists(AbstractFunctionDecl funcDecl, CallExpr call, string funcName, int arg |
59-
(
60-
// arguments to method calls...
61-
exists(string className, ClassOrStructDecl c |
62-
(
63-
// `NSRange.init`
64-
className = "NSRange" and
65-
funcName = "init(location:length:)" and
66-
arg = [0, 1]
67-
or
68-
// `NSString.character`
69-
className = ["NSString", "NSMutableString"] and
70-
funcName = "character(at:)" and
71-
arg = 0
72-
or
73-
// `NSString.character`
74-
className = ["NSString", "NSMutableString"] and
75-
funcName = "substring(from:)" and
76-
arg = 0
77-
or
78-
// `NSString.character`
79-
className = ["NSString", "NSMutableString"] and
80-
funcName = "substring(to:)" and
81-
arg = 0
82-
or
83-
// `NSMutableString.insert`
84-
className = "NSMutableString" and
85-
funcName = "insert(_:at:)" and
86-
arg = 1
87-
) and
88-
c.getName() = className and
89-
c.getABaseTypeDecl*().(ClassOrStructDecl).getAMember() = funcDecl and
90-
call.getStaticTarget() = funcDecl and
91-
flowstate = "NSString"
92-
)
93-
or
94-
// arguments to function calls...
95-
// `NSMakeRange`
96-
funcName = "NSMakeRange(_:_:)" and
97-
arg = [0, 1] and
98-
call.getStaticTarget() = funcDecl and
99-
flowstate = "NSString"
100-
or
101-
// arguments to method calls...
102-
(
103-
// `String.dropFirst`, `String.dropLast`, `String.removeFirst`, `String.removeLast`
104-
funcName = ["dropFirst(_:)", "dropLast(_:)", "removeFirst(_:)", "removeLast(_:)"] and
105-
arg = 0
106-
or
107-
// `String.prefix`, `String.suffix`
108-
funcName = ["prefix(_:)", "suffix(_:)"] and
109-
arg = 0
110-
or
111-
// `String.Index.init`
112-
funcName = "init(encodedOffset:)" and
113-
arg = 0
114-
or
115-
// `String.index`
116-
funcName = ["index(_:offsetBy:)", "index(_:offsetBy:limitBy:)"] and
117-
arg = [0, 1]
118-
or
119-
// `String.formIndex`
120-
funcName = ["formIndex(_:offsetBy:)", "formIndex(_:offsetBy:limitBy:)"] and
121-
arg = [0, 1]
122-
) and
123-
call.getStaticTarget() = funcDecl and
124-
flowstate = "String"
125-
) and
126-
// match up `funcName`, `arg`, `node`.
127-
funcDecl.getName() = funcName and
128-
call.getArgument(arg).getExpr() = node.asExpr()
129-
)
20+
flowstate = node.(StringLengthConflationSource).getStringType()
13021
}
13122

13223
override predicate isSink(DataFlow::Node node, string flowstate) {
13324
// Permit any *incorrect* flowstate, as those are the results the query
13425
// should report.
13526
exists(string correctFlowState |
136-
isSinkImpl(node, correctFlowState) and
137-
flowstate.(StringLengthConflationFlowState).getEquivClass() !=
138-
correctFlowState.(StringLengthConflationFlowState).getEquivClass()
27+
correctFlowState = node.(StringLengthConflationSink).getCorrectStringType() and
28+
flowstate.(StringType).getEquivClass() != correctFlowState.(StringType).getEquivClass()
13929
)
14030
}
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+
}
14139
}

‎swift/ql/src/queries/Security/CWE-135/StringLengthConflation.ql‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,12 @@ import DataFlow::PathGraph
1717

1818
from
1919
StringLengthConflationConfiguration config, DataFlow::PathNode source, DataFlow::PathNode sink,
20-
StringLengthConflationFlowState sourceFlowState, StringLengthConflationFlowState sinkFlowstate,
21-
string message
20+
StringType sourceType, StringType sinkType, string message
2221
where
2322
config.hasFlowPath(source, sink) and
24-
config.isSource(source.getNode(), sourceFlowState) and
25-
config.isSinkImpl(sink.getNode(), sinkFlowstate) and
23+
config.isSource(source.getNode(), sourceType) and
24+
sinkType = sink.getNode().(StringLengthConflationSink).getCorrectStringType() and
2625
message =
27-
"This " + sourceFlowState + " length is used in " + sinkFlowstate.getSingular() +
26+
"This " + sourceType + " length is used in " + sinkType.getSingular() +
2827
", but it may not be equivalent."
2928
select sink.getNode(), source, sink, message

0 commit comments

Comments
 (0)