Skip to content

Commit 3465c34

Browse files
geoffw0geoffw0
authored
Merge pull request #14680 from geoffw0/dataclosure
Swift: Generalize flow through subscript writes / test and fix some closure methods of Data
2 parents 84594e6 + b99e44c commit 3465c34

File tree

8 files changed

+141
-74
lines changed

8 files changed

+141
-74
lines changed

‎swift/ql/lib/change-notes/2023-11-03-subscript.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Generalized the data flow model for subscript writes (`a[index] = b`) so that it applies to subscripts on all kinds of objects, not just arrays.

‎swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1075,12 +1075,12 @@ predicate storeStep(Node node1, ContentSet c, Node node2) {
10751075
c.isSingleton(any(Content::CollectionContent ac))
10761076
)
10771077
or
1078-
// array assignment `a[n] = x`
1078+
// subscript assignment `a[n] = x`
10791079
exists(AssignExpr assign, SubscriptExpr subscript |
10801080
node1.asExpr() = assign.getSource() and
10811081
node2.(PostUpdateNode).getPreUpdateNode().asExpr() = subscript.getBase() and
10821082
subscript = assign.getDest() and
1083-
subscript.getBase().getType() instanceof ArrayType and
1083+
not any(DictionarySubscriptNode n).getExpr() = subscript and
10841084
c.isSingleton(any(Content::CollectionContent ac))
10851085
)
10861086
or

‎swift/ql/lib/codeql/swift/frameworks/StandardLibrary/Data.qll‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,9 +45,6 @@ private class DataSummaries extends SummaryModelCsv {
4545
";Data;true;shuffled(using:);;;Argument[-1];ReturnValue;taint",
4646
";Data;true;trimmingPrefix(_:);;;Argument[-1];ReturnValue;taint",
4747
";Data;true;trimmingPrefix(while:);;;Argument[-1];ReturnValue;taint",
48-
";Data;true;withUnsafeBytes(_:);;;Argument[-1];Argument[0].Parameter[0].CollectionElement;taint",
49-
";Data;true;withUnsafeBytes(_:);;;Argument[-1].CollectionElement;Argument[0].Parameter[0].CollectionElement;taint",
50-
";Data;true;withUnsafeBytes(_:);;;Argument[0].ReturnValue;ReturnValue;value",
5148
";Data;true;withUnsafeMutableBytes(_:);;;Argument[-1];Argument[0].Parameter[0].CollectionElement;taint",
5249
";Data;true;withUnsafeMutableBytes(_:);;;Argument[-1].CollectionElement;Argument[0].Parameter[0].CollectionElement;taint",
5350
";Data;true;withUnsafeMutableBytes(_:);;;Argument[0].Parameter[0].CollectionElement;Argument[-1].CollectionElement;value",

‎swift/ql/test/library-tests/dataflow/taint/libraries/data.swift‎

Lines changed: 104 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,13 @@ extension DataProtocol {
1717
extension UnsafeRawBufferPointer : DataProtocol { }
1818
extension Array : DataProtocol where Element == UInt8 { }
1919

20-
protocol MutableDataProtocol : DataProtocol, RangeReplaceableCollection { }
20+
protocol MutableDataProtocol : DataProtocol, MutableCollection, RangeReplaceableCollection { }
2121

22-
struct Data : MutableDataProtocol
22+
protocol ContiguousBytes {
23+
func withUnsafeBytes<R>(_ body: (UnsafeRawBufferPointer) throws -> R) rethrows -> R
24+
}
25+
26+
struct Data : MutableDataProtocol, ContiguousBytes
2327
{
2428
struct Base64EncodingOptions : OptionSet { let rawValue: Int }
2529
struct Base64DecodingOptions : OptionSet { let rawValue: Int }
@@ -31,7 +35,10 @@ struct Data : MutableDataProtocol
3135
var endIndex: Self.Index { get { return 0 } }
3236
func index(after: Self.Index) -> Self.Index { return 0 }
3337
func index(before: Self.Index) -> Self.Index { return 0 }
34-
subscript(position: Self.Index) -> Self.Element { get { return 0 } }
38+
subscript(position: Self.Index) -> Self.Element {
39+
get { return 0 }
40+
set { }
41+
}
3542

3643
init() {}
3744
init<S>(_ elements: S) {}
@@ -74,6 +81,9 @@ struct Data : MutableDataProtocol
7481
func shuffled<T>(using: inout T) -> [UInt8] { return [] }
7582
func trimmingPrefix<Prefix>(_ prefix: Prefix) -> Data where Prefix : Sequence, UInt8 == Prefix.Element { return Data("") }
7683
func trimmingPrefix(while: (UInt8) -> Bool) -> Data { return Data("") }
84+
85+
mutating func withUnsafeMutableBytes<ResultType>(_ body: (UnsafeMutableRawBufferPointer) throws -> ResultType) rethrows -> ResultType { return try body(UnsafeMutableRawBufferPointer(start: nil, count: 0)) }
86+
func withUnsafeBytes<R>(_ body: (UnsafeRawBufferPointer) throws -> R) rethrows -> R { return try body(UnsafeRawBufferPointer(start: nil, count: 0)) }
7787
}
7888

7989
// --- tests ---
@@ -94,193 +104,241 @@ func taintThroughData() {
94104
let dataTainted2 = Data(dataTainted)
95105

96106
sink(arg: dataClean)
97-
sink(arg: dataTainted) // $ tainted=93
98-
sink(arg: dataTainted2) // $ tainted=93
107+
sink(arg: dataTainted) // $ tainted=103
108+
sink(arg: dataTainted2) // $ tainted=103
99109

100110
// ";Data;true;init(base64Encoded:options:);;;Argument[0];ReturnValue;taint",
101111
let dataTainted3 = Data(base64Encoded: source() as! Data, options: [])
102-
sink(arg: dataTainted3) // $ tainted=101
112+
sink(arg: dataTainted3) // $ tainted=111
103113

104114
// ";Data;true;init(buffer:);;;Argument[0];ReturnValue;taint",
105115
let dataTainted4 = Data(buffer: source() as! UnsafeBufferPointer<UInt8>)
106-
sink(arg: dataTainted4) // $ tainted=105
116+
sink(arg: dataTainted4) // $ tainted=115
107117
let dataTainted5 = Data(buffer: source() as! UnsafeMutablePointer<UInt8>)
108-
sink(arg: dataTainted5) // $ tainted=107
118+
sink(arg: dataTainted5) // $ tainted=117
109119

110120
// ";Data;true;init(bytes:count:);;;Argument[0];ReturnValue;taint",
111121
let dataTainted6 = Data(bytes: source() as! UnsafeRawPointer, count: 0)
112-
sink(arg: dataTainted6) // $ tainted=111
122+
sink(arg: dataTainted6) // $ tainted=121
113123

114124
// ";Data;true;init(bytesNoCopy:count:deallocator:);;;Argument[0];ReturnValue;taint",
115125
let dataTainted7 = Data(bytesNoCopy: source() as! UnsafeRawPointer, count: 0, deallocator: Data.Deallocator.none)
116-
sink(arg: dataTainted7) // $ tainted=115
126+
sink(arg: dataTainted7) // $ tainted=125
117127

118128
// ";Data;true;init(contentsOf:options:);;;Argument[0];ReturnValue;taint",
119129
let urlTainted8 = source() as! URL
120130
let dataTainted8 = Data(contentsOf: urlTainted8, options: [])
121-
sink(arg: dataTainted8) // $ tainted=119
131+
sink(arg: dataTainted8) // $ tainted=129
122132

123133
// ";Data;true;init(referencing:);;;Argument[0];ReturnValue;taint",
124134
let dataTainted9 = Data(referencing: source() as! NSData)
125-
sink(arg: dataTainted9) // $ tainted=124
135+
sink(arg: dataTainted9) // $ tainted=134
126136

127137
// ";Data;true;append(_:);;;Argument[0];Argument[-1];taint",
128138
let dataTainted10 = Data("")
129139
dataTainted10.append(source() as! Data)
130-
sink(arg: dataTainted10) // $ tainted=129
140+
sink(arg: dataTainted10) // $ tainted=139
131141

132142
let dataTainted11 = Data("")
133143
dataTainted11.append(source() as! UInt8)
134-
sink(arg: dataTainted11) // $ tainted=133
144+
sink(arg: dataTainted11) // $ tainted=143
135145

136146
let dataTainted12 = Data("")
137147
dataTainted12.append(source() as! UnsafeBufferPointer<UInt8>)
138-
sink(arg: dataTainted12) // $ tainted=137
148+
sink(arg: dataTainted12) // $ tainted=147
139149

140150
// ";Data;true;append(_:count:);;;Argument[0];Argument[-1];taint",
141151
let dataTainted13 = Data("")
142152
dataTainted13.append(source() as! UnsafePointer<UInt8>, count: 0)
143-
sink(arg: dataTainted13) // $ tainted=142
153+
sink(arg: dataTainted13) // $ tainted=152
144154

145155
// ";Data;true;append(contentsOf:);;;Argument[0];Argument[-1];taint",
146156
let dataTainted14 = Data("")
147157
dataTainted14.append(contentsOf: source() as! [UInt8])
148-
sink(arg: dataTainted14) // $ tainted=147
158+
sink(arg: dataTainted14) // $ tainted=157
149159

150160
// ";Data;true;base64EncodedData(options:);;;Argument[-1];ReturnValue;taint",
151161
let dataTainted15 = source() as! Data
152-
sink(arg: dataTainted15.base64EncodedData(options: [])) // $ tainted=151
162+
sink(arg: dataTainted15.base64EncodedData(options: [])) // $ tainted=161
153163

154164
// ";Data;true;base64EncodedString(options:);;;Argument[-1];ReturnValue;taint",
155165
let dataTainted16 = source() as! Data
156-
sink(arg: dataTainted16.base64EncodedString(options: [])) // $ tainted=155
166+
sink(arg: dataTainted16.base64EncodedString(options: [])) // $ tainted=165
157167

158168
// ";Data;true;compactMap(_:);;;Argument[-1];ReturnValue;taint",
159169
let dataTainted17 = source() as! Data
160170
let compactMapped: [Int] = dataTainted17.compactMap { str in Int(str) }
161-
sink(arg: compactMapped) // $ tainted=159
171+
sink(arg: compactMapped) // $ tainted=169
162172

163173
// ";Data;true;copyBytes(to:);;;Argument[-1];Argument[0];taint",
164174
let dataTainted18 = source() as! Data
165175
let pointerTainted18 = UnsafeMutableRawBufferPointer.allocate(byteCount: 0, alignment: 0)
166176
dataTainted18.copyBytes(to: pointerTainted18)
167-
sink(arg: pointerTainted18) // $ tainted=164
177+
sink(arg: pointerTainted18) // $ tainted=174
168178

169179
// ";Data;true;copyBytes(to:count:);;;Argument[-1];Argument[0];taint",
170180
let dataTainted19 = source() as! Data
171181
let pointerTainted19 = UnsafeMutablePointer<UInt8>.allocate(capacity: 0)
172182
dataTainted19.copyBytes(to: pointerTainted19, count: 0)
173-
sink(arg: pointerTainted19) // $ tainted=170
183+
sink(arg: pointerTainted19) // $ tainted=180
174184

175185
// ";Data;true;copyBytes(to:from:);;;Argument[-1];Argument[0];taint",
176186
let dataTainted20 = source() as! Data
177187
let pointerTainted20 = UnsafeMutablePointer<UInt8>.allocate(capacity: 0)
178188
dataTainted20.copyBytes(to: pointerTainted20, from: 0..<1)
179-
sink(arg: pointerTainted20) // $ tainted=176
189+
sink(arg: pointerTainted20) // $ tainted=186
180190

181191
// ";Data;true;flatMap(_:);;;Argument[-1];ReturnValue;taint",
182192
let dataTainted21 = source() as! Data
183193
let flatMapped = dataTainted21.flatMap { Array(repeating: $0, count: 0) }
184-
sink(arg: flatMapped) // $ tainted=182
194+
sink(arg: flatMapped) // $ tainted=192
185195

186196
let dataTainted22 = source() as! Data
187197
let flatMapped2 = dataTainted22.flatMap { str in Int(str) }
188-
sink(arg: flatMapped2) // $ tainted=186
198+
sink(arg: flatMapped2) // $ tainted=196
189199

190200
// ";Data;true;insert(_:at:);;;Argument[0];Argument[-1];taint",
191201
let dataTainted23 = Data("")
192202
dataTainted23.insert(source() as! UInt8, at: 0)
193-
sink(arg: dataTainted23) // $ tainted=192
203+
sink(arg: dataTainted23) // $ tainted=202
194204

195205
// ";Data;true;insert(contentsOf:at:);;;Argument[0];Argument[-1];taint",
196206
let dataTainted24 = Data("")
197207
dataTainted24.insert(contentsOf: source() as! [UInt8], at: 0)
198-
sink(arg: dataTainted24) // $ tainted=197
208+
sink(arg: dataTainted24) // $ tainted=207
199209

200210
// ";Data;true;map(_:);;;Argument[-1];ReturnValue;taint",
201211
let dataTainted25 = source() as! Data
202212
let mapped = dataTainted25.map { $0 }
203-
sink(arg: mapped) // $ tainted=201
213+
sink(arg: mapped) // $ tainted=211
204214

205215
// ";Data;true;reduce(into:_:);;;Argument[-1];ReturnValue;taint",
206216
let dataTainted26 = source() as! Data
207217
let reduced = dataTainted26.reduce(into: [:]) { c, i in c[i, default: 0] += 1 }
208-
sink(arg: reduced) // $ tainted=206
218+
sink(arg: reduced) // $ tainted=216
209219

210220
// ";Data;true;replace(_:with:maxReplacements:);;;Argument[1];Argument[-1];taint",
211221
let dataTainted27 = Data("")
212222
dataTainted27.replace([0], with: source() as! [UInt8], maxReplacements: .max)
213-
sink(arg: dataTainted27) // $ tainted=212
223+
sink(arg: dataTainted27) // $ tainted=222
214224

215225
// ";Data;true;replaceSubrange(_:with:);;;Argument[1];Argument[-1];taint",
216226
let dataTainted28 = Data("")
217227
dataTainted28.replaceSubrange(1..<3, with: source() as! Data)
218-
sink(arg: dataTainted28) // $ tainted=217
228+
sink(arg: dataTainted28) // $ tainted=227
219229

220230
let dataTainted29 = Data("")
221231
dataTainted29.replaceSubrange(1..<3, with: source() as! [UInt8])
222-
sink(arg: dataTainted29) // $ tainted=221
232+
sink(arg: dataTainted29) // $ tainted=231
223233

224234
let dataTainted30 = Data("")
225235
dataTainted30.replaceSubrange(1..<3, with: source() as! UnsafeBufferPointer<UInt8>)
226-
sink(arg: dataTainted30) // $ tainted=225
236+
sink(arg: dataTainted30) // $ tainted=235
227237

228238
// ";Data;true;replaceSubrange(_:with:count:);;;Argument[1];Argument[-1];taint",
229239
let dataTainted31 = Data("")
230240
dataTainted31.replaceSubrange(1..<3, with: source() as! UnsafeRawPointer, count: 0)
231-
sink(arg: dataTainted31) // $ tainted=230
241+
sink(arg: dataTainted31) // $ tainted=240
232242

233243
// ";Data;true;replacing(_:with:maxReplacements:);;;Argument[1];Argument[-1];taint",
234244
let dataTainted32 = Data("")
235245
let _ = dataTainted32.replacing([0], with: source() as! [UInt8], maxReplacements: 0)
236-
sink(arg: dataTainted32) // $ tainted=235
246+
sink(arg: dataTainted32) // $ tainted=245
237247

238248
// ";Data;true;replacing(_:with:subrange:maxReplacements:);;;Argument[1];Argument[-1];taint",
239249
let dataTainted33 = Data("")
240250
let _ = dataTainted33.replacing([0], with: source() as! [UInt8], subrange: 1..<3, maxReplacements: 0)
241-
sink(arg: dataTainted33) // $ tainted=240
251+
sink(arg: dataTainted33) // $ tainted=250
242252

243253
// ";Data;true;reversed();;;Argument[-1];ReturnValue;taint",
244254
let dataTainted34 = source() as! Data
245-
sink(arg: dataTainted34.reversed()) // $ tainted=244
255+
sink(arg: dataTainted34.reversed()) // $ tainted=254
246256

247257
// ";Data;true;sorted();;;Argument[-1];ReturnValue;taint",
248258
let dataTainted35 = source() as! Data
249-
sink(arg: dataTainted35.sorted()) // $ tainted=248
259+
sink(arg: dataTainted35.sorted()) // $ tainted=258
250260

251261
// ";Data;true;sorted(by:);;;Argument[-1];ReturnValue;taint",
252262
let dataTainted36 = source() as! Data
253-
sink(arg: dataTainted36.sorted{ _,_ in return false }) // $ tainted=252
263+
sink(arg: dataTainted36.sorted{ _,_ in return false }) // $ tainted=262
254264

255265
// ";Data;true;sorted(using:);;;Argument[-1];ReturnValue;taint",
256266
let dataTainted37 = source() as! Data
257-
sink(arg: dataTainted37.sorted(using: cmp()!)) // $ tainted=256
267+
sink(arg: dataTainted37.sorted(using: cmp()!)) // $ tainted=266
258268

259269
// ";Data;true;shuffled();;;Argument[-1];ReturnValue;taint",
260270
let dataTainted38 = source() as! Data
261-
sink(arg: dataTainted38.shuffled()) // $ tainted=260
271+
sink(arg: dataTainted38.shuffled()) // $ tainted=270
262272

263273
// ";Data;true;shuffled(using:);;;Argument[-1];ReturnValue;taint",
264274
let dataTainted39 = source() as! Data
265275
var myRng = rng()!
266-
sink(arg: dataTainted39.shuffled(using: &myRng)) // $ tainted=264
276+
sink(arg: dataTainted39.shuffled(using: &myRng)) // $ tainted=274
267277

268278
// ";Data;true;trimmingPrefix(_:);;;Argument[-1];ReturnValue;taint",
269279
let dataTainted40 = source() as! Data
270-
sink(arg: dataTainted40.trimmingPrefix([0])) // $ tainted=269
280+
sink(arg: dataTainted40.trimmingPrefix([0])) // $ tainted=279
271281

272282
// ";Data;true;trimmingPrefix(while:);;;Argument[-1];ReturnValue;taint"
273283
let dataTainted41 = source() as! Data
274-
sink(arg: dataTainted41.trimmingPrefix { _ in false }) // $ tainted=273
284+
sink(arg: dataTainted41.trimmingPrefix { _ in false }) // $ tainted=283
275285

276286
// ";DataProtocol;true;copyBytes(to:);;;Argument[-1];Argument[0];taint",
277287
let dataTainted43 = source() as! UnsafeRawBufferPointer
278288
let pointerTainted43 = UnsafeMutableRawBufferPointer.allocate(byteCount: 0, alignment: 0)
279289
dataTainted43.copyBytes(to: pointerTainted43)
280-
sink(arg: pointerTainted43) // $ tainted=277
290+
sink(arg: pointerTainted43) // $ tainted=287
281291

282292
let dataTainted44 = source() as! Array<UInt8>
283293
let pointerTainted44 = UnsafeMutableRawBufferPointer.allocate(byteCount: 0, alignment: 0)
284294
dataTainted44.copyBytes(to: pointerTainted44)
285-
sink(arg: pointerTainted44) // $ tainted=282
295+
sink(arg: pointerTainted44) // $ tainted=292
296+
297+
let dataTainted45 = source() as! Data
298+
let result45 = dataTainted45.withContiguousStorageIfAvailable({
299+
buffer in
300+
301+
sink(arg: buffer) // $ tainted=297
302+
sink(arg: buffer[0]) // $ tainted=297
303+
304+
return source() as! Int
305+
})
306+
sink(arg: result45!) // $ tainted=304
307+
308+
var data46 = Data(0)
309+
let result46 = data46.withContiguousMutableStorageIfAvailable({
310+
buffer in
311+
312+
buffer[0] = source() as! UInt8
313+
sink(arg: buffer) // $ tainted=312
314+
sink(arg: buffer[0]) // $ tainted=312
315+
316+
return source() as! Int
317+
})
318+
sink(arg: result46!) // $ tainted=316
319+
sink(arg: data46) // $ tainted=312
320+
321+
let dataTainted47 = source() as! Data
322+
let result47 = dataTainted47.withUnsafeBytes({
323+
buffer in
324+
325+
sink(arg: buffer) // $ tainted=321
326+
sink(arg: buffer[0]) // $ tainted=321
327+
328+
return source() as! Int
329+
})
330+
sink(arg: result47) // $ tainted=328
331+
332+
var data48 = Data(0)
333+
let result48 = data48.withUnsafeMutableBytes({
334+
buffer in
335+
336+
buffer[0] = source() as! UInt8
337+
sink(arg: buffer) // $ tainted=336
338+
sink(arg: buffer[0]) // $ tainted=336
339+
340+
return source() as! Int
341+
})
342+
sink(arg: result48) // $ tainted=340
343+
sink(arg: data48) // $ tainted=336
286344
}

‎swift/ql/test/library-tests/dataflow/taint/libraries/int.swift‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -151,12 +151,12 @@ func taintCollections(array: inout Array<Int>, contiguousArray: inout Contiguous
151151
})
152152

153153
contiguousArray[0] = source2()
154-
sink(arg: contiguousArray)
155-
sink(arg: contiguousArray[0]) // $ MISSING: tainted=153
154+
sink(arg: contiguousArray) // $ tainted=153
155+
sink(arg: contiguousArray[0]) // $ tainted=153
156156
contiguousArray.withContiguousStorageIfAvailable({
157157
buffer in
158-
sink(arg: buffer)
159-
sink(arg: buffer[0]) // $ MISSING: tainted=153
158+
sink(arg: buffer) // $ tainted=153
159+
sink(arg: buffer[0]) // $ tainted=153
160160
})
161161

162162
dictionary[0] = source2()

‎swift/ql/test/library-tests/dataflow/taint/libraries/unsafepointer.swift‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,8 @@ func taintBuffer(buffer: UnsafeMutableBufferPointer<UInt8>) {
5959

6060
buffer[0] = sourceUInt8()
6161

62-
sink(arg: buffer[0]) // $ MISSING: tainted=60
63-
sink(arg: buffer)
62+
sink(arg: buffer[0]) // $ tainted=60
63+
sink(arg: buffer) // $ tainted=60
6464
}
6565

6666
func testMutatingBufferInCall(ptr: UnsafeMutablePointer<UInt8>) {
@@ -71,8 +71,8 @@ func testMutatingBufferInCall(ptr: UnsafeMutablePointer<UInt8>) {
7171

7272
taintBuffer(buffer: buffer) // mutates `buffer` contents with a tainted value
7373

74-
sink(arg: buffer[0]) // $ MISSING: tainted=60
75-
sink(arg: buffer)
74+
sink(arg: buffer[0]) // $ tainted=60
75+
sink(arg: buffer) // $ tainted=60
7676

7777
}
7878

0 commit comments

Comments
 (0)