Skip to content

Commit 25354d2

Browse files
atorralbaatorralba
committed
Apply code review suggestions
1 parent 6a8b9fd commit 25354d2

File tree

2 files changed

+78
-52
lines changed
  • swift/ql
    • lib/codeql/swift/frameworks/StandardLibrary
    • test/library-tests/dataflow/taint

2 files changed

+78
-52
lines changed

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,13 @@ private class DataSummaries extends SummaryModelCsv {
3737
";Data;true;replaceSubrange(_:with:count:);;;Argument[1];Argument[-1];taint",
3838
";Data;true;replacing(_:with:maxReplacements:);;;Argument[1];Argument[-1];taint",
3939
";Data;true;replacing(_:with:subrange:maxReplacements:);;;Argument[1];Argument[-1];taint",
40+
// TODO: this should be implemented by a model of BidirectionalCollection
41+
// ";Data;true;reversed();;;Argument[-1];ReturnValue;taint",
42+
";Data;true;sorted();;;Argument[-1];ReturnValue;taint",
43+
";Data;true;sorted(by:);;;Argument[-1];ReturnValue;taint",
44+
";Data;true;sorted(using:);;;Argument[-1];ReturnValue;taint",
4045
";Data;true;shuffled();;;Argument[-1];ReturnValue;taint",
4146
";Data;true;shuffled(using:);;;Argument[-1];ReturnValue;taint",
42-
";Data;true;sorted(using:);;;Argument[-1];ReturnValue;taint",
4347
";Data;true;trimmingPrefix(_:);;;Argument[-1];ReturnValue;taint",
4448
";Data;true;trimmingPrefix(while:);;;Argument[-1];ReturnValue;taint"
4549
]

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

Lines changed: 73 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,21 @@ class NSData {}
66
protocol SortComparator {
77
associatedtype Compared
88
}
9-
struct Data
9+
10+
struct Data : BidirectionalCollection
1011
{
1112
struct Base64EncodingOptions : OptionSet { let rawValue: Int }
1213
struct Base64DecodingOptions : OptionSet { let rawValue: Int }
13-
enum Deallocator { case none }
1414
struct ReadingOptions : OptionSet { let rawValue: Int }
15+
enum Deallocator { case none }
1516
typealias Index = Int
17+
typealias Element = UInt8
18+
var startIndex: Self.Index { get { return 0 } }
19+
var endIndex: Self.Index { get { return 0 } }
20+
func index(after: Self.Index) -> Self.Index { return 0 }
21+
func index(before: Self.Index) -> Self.Index { return 0 }
22+
subscript(position: Self.Index) -> Self.Element { get { return 0 } }
23+
1624
init<S>(_ elements: S) {}
1725
init(base64Encoded: Data, options: Data.Base64DecodingOptions) {}
1826
init<SourceType>(buffer: UnsafeBufferPointer<SourceType>) {}
@@ -47,9 +55,11 @@ struct Data
4755
func replaceSubrange<C, R>(_: R, with: C) where C : Collection, R : RangeExpression, UInt8 == C.Element, Int == R.Bound {}
4856
func replacing<C, Replacement>(_: C, with: Replacement, maxReplacements: Int = .max) -> Data where C : Collection, Replacement : Collection, UInt8 == C.Element, C.Element == Replacement.Element { return Data("") }
4957
func replacing<C, Replacement>(_: C, with: Replacement, subrange: Range<Int>, maxReplacements: Int = .max) -> Data where C : Collection, Replacement : Collection, UInt8 == C.Element, C.Element == Replacement.Element { return Data("") }
58+
func sorted() -> [UInt8] { return [] }
59+
func sorted(by: (UInt8, UInt8) throws -> Bool) rethrows -> [UInt8] { return [] }
60+
func sorted<Comparator>(using: Comparator) -> [UInt8] where Comparator : SortComparator, UInt8 == Comparator.Compared { return [] }
5061
func shuffled() -> [UInt8] { return [] }
5162
func shuffled<T>(using: inout T) -> [UInt8] { return [] }
52-
func sorted<Comparator>(using: Comparator) -> [UInt8] where Comparator : SortComparator, UInt8 == Comparator.Compared { return [] }
5363
func trimmingPrefix<Prefix>(_ prefix: Prefix) -> Data where Prefix : Sequence, UInt8 == Prefix.Element { return Data("") }
5464
func trimmingPrefix(while: (UInt8) -> Bool) -> Data { return Data("") }
5565
}
@@ -72,170 +82,182 @@ func taintThroughData() {
7282
let dataTainted2 = Data(dataTainted)
7383

7484
sink(arg: dataClean)
75-
sink(arg: dataTainted) // $ MISSING: tainted=71
76-
sink(arg: dataTainted2) // $ MISSING: tainted=71
85+
sink(arg: dataTainted) // $ MISSING: tainted=81
86+
sink(arg: dataTainted2) // $ MISSING: tainted=81
7787

7888
// ";Data;true;init(base64Encoded:options:);;;Argument[0];ReturnValue;taint",
7989
let dataTainted3 = Data(base64Encoded: source() as! Data, options: [])
80-
sink(arg: dataTainted3) // $ tainted=79
90+
sink(arg: dataTainted3) // $ tainted=89
8191

8292
// ";Data;true;init(buffer:);;;Argument[0];ReturnValue;taint",
8393
let dataTainted4 = Data(buffer: source() as! UnsafeBufferPointer<UInt8>)
84-
sink(arg: dataTainted4) // $ tainted=83
94+
sink(arg: dataTainted4) // $ tainted=93
8595
let dataTainted5 = Data(buffer: source() as! UnsafeMutablePointer<UInt8>)
86-
sink(arg: dataTainted5) // $ tainted=85
96+
sink(arg: dataTainted5) // $ tainted=95
8797

8898
// ";Data;true;init(bytes:count:);;;Argument[0];ReturnValue;taint",
8999
let dataTainted6 = Data(bytes: source() as! UnsafeRawPointer, count: 0)
90-
sink(arg: dataTainted6) // $ tainted=89
100+
sink(arg: dataTainted6) // $ tainted=99
91101

92102
// ";Data;true;init(bytesNoCopy:count:deallocator:);;;Argument[0];ReturnValue;taint",
93103
let dataTainted7 = Data(bytesNoCopy: source() as! UnsafeRawPointer, count: 0, deallocator: Data.Deallocator.none)
94-
sink(arg: dataTainted7) // $ tainted=93
104+
sink(arg: dataTainted7) // $ tainted=103
95105

96106
// ";Data;true;init(contentsOf:options:);;;Argument[0];ReturnValue;taint",
97-
let dataTainted8 = Data(contentsOf: source() as! URL, options: [])
98-
sink(arg: dataTainted8) // $ tainted=97
107+
let urlTainted8 = source() as! URL
108+
let dataTainted8 = Data(contentsOf: urlTainted8, options: [])
109+
sink(arg: dataTainted8) // $ tainted=107
99110

100111
// ";Data;true;init(referencing:);;;Argument[0];ReturnValue;taint",
101112
let dataTainted9 = Data(referencing: source() as! NSData)
102-
sink(arg: dataTainted9) // $ tainted=101
113+
sink(arg: dataTainted9) // $ tainted=112
103114

104115
// ";Data;true;append(_:);;;Argument[0];Argument[-1];taint",
105116
let dataTainted10 = Data("")
106117
dataTainted10.append(source() as! Data)
107-
sink(arg: dataTainted10) // $ tainted=106
118+
sink(arg: dataTainted10) // $ tainted=117
108119

109120
let dataTainted11 = Data("")
110121
dataTainted11.append(source() as! UInt8)
111-
sink(arg: dataTainted11) // $ tainted=110
122+
sink(arg: dataTainted11) // $ tainted=121
112123

113124
let dataTainted12 = Data("")
114125
dataTainted12.append(source() as! UnsafeBufferPointer<UInt8>)
115-
sink(arg: dataTainted12) // $ tainted=114
126+
sink(arg: dataTainted12) // $ tainted=125
116127

117128
// ";Data;true;append(_:count:);;;Argument[0];Argument[-1];taint",
118129
let dataTainted13 = Data("")
119130
dataTainted13.append(source() as! UnsafePointer<UInt8>, count: 0)
120-
sink(arg: dataTainted13) // $ tainted=119
131+
sink(arg: dataTainted13) // $ tainted=130
121132

122133
// ";Data;true;append(contentsOf:);;;Argument[0];Argument[-1];taint",
123134
let dataTainted14 = Data("")
124135
dataTainted14.append(contentsOf: source() as! [UInt8])
125-
sink(arg: dataTainted14) // $ tainted=124
136+
sink(arg: dataTainted14) // $ tainted=135
126137

127138
// ";Data;true;base64EncodedData(options:);;;Argument[-1];ReturnValue;taint",
128139
let dataTainted15 = source() as! Data
129-
sink(arg: dataTainted15.base64EncodedData(options: [])) // $ tainted=128
140+
sink(arg: dataTainted15.base64EncodedData(options: [])) // $ tainted=139
130141

131142
// ";Data;true;base64EncodedString(options:);;;Argument[-1];ReturnValue;taint",
132143
let dataTainted16 = source() as! Data
133-
sink(arg: dataTainted16.base64EncodedString(options: [])) // $ tainted=132
144+
sink(arg: dataTainted16.base64EncodedString(options: [])) // $ tainted=143
134145

135146
// ";Data;true;compactMap(_:);;;Argument[-1];ReturnValue;taint",
136147
let dataTainted17 = source() as! Data
137148
let compactMapped: [Int] = dataTainted17.compactMap { str in Int(str) }
138-
sink(arg: compactMapped) // $ tainted=136
149+
sink(arg: compactMapped) // $ tainted=147
139150

140151
// ";Data;true;copyBytes(to:);;;Argument[-1];Argument[0];taint",
141152
let dataTainted18 = source() as! Data
142153
let pointerTainted18 = UnsafeMutableRawBufferPointer.allocate(byteCount: 0, alignment: 0)
143154
dataTainted18.copyBytes(to: pointerTainted18)
144-
sink(arg: pointerTainted18) // $ tainted=141
155+
sink(arg: pointerTainted18) // $ tainted=152
145156

146157
// ";Data;true;copyBytes(to:count:);;;Argument[-1];Argument[0];taint",
147158
let dataTainted19 = source() as! Data
148159
let pointerTainted19 = UnsafeMutablePointer<UInt8>.allocate(capacity: 0)
149160
dataTainted19.copyBytes(to: pointerTainted19, count: 0)
150-
sink(arg: pointerTainted19) // $ MISSING: tainted=147
161+
sink(arg: pointerTainted19) // $ MISSING: tainted=158
151162

152163
// ";Data;true;copyBytes(to:from:);;;Argument[-1];Argument[0];taint",
153164
let dataTainted20 = source() as! Data
154165
let pointerTainted20 = UnsafeMutablePointer<UInt8>.allocate(capacity: 0)
155166
dataTainted20.copyBytes(to: pointerTainted20, from: 0..<1)
156-
sink(arg: pointerTainted20) // $ MISSING: tainted=153
167+
sink(arg: pointerTainted20) // $ MISSING: tainted=164
157168

158169
// ";Data;true;flatMap(_:);;;Argument[-1];ReturnValue;taint",
159170
let dataTainted21 = source() as! Data
160171
let flatMapped = dataTainted21.flatMap { Array(repeating: $0, count: 0) }
161-
sink(arg: flatMapped) // $ tainted=159
172+
sink(arg: flatMapped) // $ tainted=170
162173

163174
let dataTainted22 = source() as! Data
164175
let flatMapped2 = dataTainted22.flatMap { str in Int(str) }
165-
sink(arg: flatMapped2) // $ tainted=163
176+
sink(arg: flatMapped2) // $ tainted=174
166177

167178
// ";Data;true;insert(_:at:);;;Argument[0];Argument[-1];taint",
168179
let dataTainted23 = Data("")
169180
dataTainted23.insert(source() as! UInt8, at: 0)
170-
sink(arg: dataTainted23) // $ tainted=169
181+
sink(arg: dataTainted23) // $ tainted=180
171182

172183
// ";Data;true;insert(contentsOf:at:);;;Argument[0];Argument[-1];taint",
173184
let dataTainted24 = Data("")
174185
dataTainted24.insert(contentsOf: source() as! [UInt8], at: 0)
175-
sink(arg: dataTainted24) // $ tainted=174
186+
sink(arg: dataTainted24) // $ tainted=185
176187

177188
// ";Data;true;map(_:);;;Argument[-1];ReturnValue;taint",
178189
let dataTainted25 = source() as! Data
179190
let mapped = dataTainted25.map { $0 }
180-
sink(arg: mapped) // $ tainted=178
191+
sink(arg: mapped) // $ tainted=189
181192

182193
// ";Data;true;reduce(into:_:);;;Argument[-1];ReturnValue;taint",
183194
let dataTainted26 = source() as! Data
184195
let reduced = dataTainted26.reduce(into: [:]) { c, i in c[i, default: 0] += 1 }
185-
sink(arg: reduced) // $ tainted=183
196+
sink(arg: reduced) // $ tainted=194
186197

187198
// ";Data;true;replace(_:with:maxReplacements:);;;Argument[1];Argument[-1];taint",
188199
let dataTainted27 = Data("")
189200
dataTainted27.replace([0], with: source() as! [UInt8], maxReplacements: .max)
190-
sink(arg: dataTainted27) // $ tainted=189
201+
sink(arg: dataTainted27) // $ tainted=200
191202

192203
// ";Data;true;replaceSubrange(_:with:);;;Argument[1];Argument[-1];taint",
193204
let dataTainted28 = Data("")
194205
dataTainted28.replaceSubrange(1..<3, with: source() as! Data)
195-
sink(arg: dataTainted28) // $ tainted=194
206+
sink(arg: dataTainted28) // $ tainted=205
196207

197208
let dataTainted29 = Data("")
198209
dataTainted29.replaceSubrange(1..<3, with: source() as! [UInt8])
199-
sink(arg: dataTainted29) // $ tainted=198
210+
sink(arg: dataTainted29) // $ tainted=209
200211

201212
let dataTainted30 = Data("")
202213
dataTainted30.replaceSubrange(1..<3, with: source() as! UnsafeBufferPointer<UInt8>)
203-
sink(arg: dataTainted30) // $ tainted=202
214+
sink(arg: dataTainted30) // $ tainted=213
204215

205216
// ";Data;true;replaceSubrange(_:with:count:);;;Argument[1];Argument[-1];taint",
206217
let dataTainted31 = Data("")
207218
dataTainted31.replaceSubrange(1..<3, with: source() as! UnsafeRawPointer, count: 0)
208-
sink(arg: dataTainted31) // $ tainted=207
219+
sink(arg: dataTainted31) // $ tainted=218
209220

210221
// ";Data;true;replacing(_:with:maxReplacements:);;;Argument[1];Argument[-1];taint",
211222
let dataTainted32 = Data("")
212-
dataTainted32.replacing([0], with: source() as! [UInt8], maxReplacements: 0)
213-
sink(arg: dataTainted32) // $ tainted=212
223+
let _ = dataTainted32.replacing([0], with: source() as! [UInt8], maxReplacements: 0)
224+
sink(arg: dataTainted32) // $ tainted=223
214225

215226
// ";Data;true;replacing(_:with:subrange:maxReplacements:);;;Argument[1];Argument[-1];taint",
216227
let dataTainted33 = Data("")
217-
dataTainted33.replacing([0], with: source() as! [UInt8], subrange: 1..<3, maxReplacements: 0)
218-
sink(arg: dataTainted33) // $ tainted=217
228+
let _ = dataTainted33.replacing([0], with: source() as! [UInt8], subrange: 1..<3, maxReplacements: 0)
229+
sink(arg: dataTainted33) // $ tainted=228
219230

220-
// ";Data;true;shuffled();;;Argument[-1];ReturnValue;taint",
231+
// ";Data;true;reversed();;;Argument[-1];ReturnValue;taint",
221232
let dataTainted34 = source() as! Data
222-
sink(arg: dataTainted34.shuffled()) // $ tainted=221
223-
224-
// ";Data;true;shuffled(using:);;;Argument[-1];ReturnValue;taint",
233+
sink(arg: dataTainted34.reversed()) // $ MISSING: tainted=232 // Needs models for BidirectionalCollection
234+
235+
// ";Data;true;sorted();;;Argument[-1];ReturnValue;taint",
225236
let dataTainted35 = source() as! Data
226-
var rng = rng()!
227-
sink(arg: dataTainted35.shuffled(using: &rng)) // $ tainted=225
237+
sink(arg: dataTainted35.sorted()) // $ tainted=236
228238

229-
// ";Data;true;sorted(using:);;;Argument[-1];ReturnValue;taint",
239+
// ";Data;true;sorted(by:);;;Argument[-1];ReturnValue;taint",
230240
let dataTainted36 = source() as! Data
231-
sink(arg: dataTainted36.sorted(using: cmp()!)) // $ tainted=230
241+
sink(arg: dataTainted36.sorted{ _,_ in return false }) // $ tainted=240
232242

233-
// ";Data;true;trimmingPrefix(_:);;;Argument[-1];ReturnValue;taint",
243+
// ";Data;true;sorted(using:);;;Argument[-1];ReturnValue;taint",
234244
let dataTainted37 = source() as! Data
235-
sink(arg: dataTainted37.trimmingPrefix([0])) // $ tainted=234
245+
sink(arg: dataTainted37.sorted(using: cmp()!)) // $ tainted=244
236246

237-
// ";Data;true;trimmingPrefix(while:);;;Argument[-1];ReturnValue;taint"
247+
// ";Data;true;shuffled();;;Argument[-1];ReturnValue;taint",
238248
let dataTainted38 = source() as! Data
239-
sink(arg: dataTainted38.trimmingPrefix { _ in false }) // $ tainted=238
249+
sink(arg: dataTainted38.shuffled()) // $ tainted=248
250+
251+
// ";Data;true;shuffled(using:);;;Argument[-1];ReturnValue;taint",
252+
let dataTainted39 = source() as! Data
253+
var rng = rng()!
254+
sink(arg: dataTainted39.shuffled(using: &rng)) // $ tainted=252
240255

256+
// ";Data;true;trimmingPrefix(_:);;;Argument[-1];ReturnValue;taint",
257+
let dataTainted40 = source() as! Data
258+
sink(arg: dataTainted40.trimmingPrefix([0])) // $ tainted=257
259+
260+
// ";Data;true;trimmingPrefix(while:);;;Argument[-1];ReturnValue;taint"
261+
let dataTainted41 = source() as! Data
262+
sink(arg: dataTainted41.trimmingPrefix { _ in false }) // $ tainted=261
241263
}

0 commit comments

Comments
 (0)