Skip to content

Commit 81d77bf

Browse files
rdmarsh2rdmarsh2
authored
Merge pull request #14578 from geoffw0/stringwith
Swift: Models for String methods involving closures.
2 parents a35bda2 + bf50384 commit 81d77bf

File tree

3 files changed

+110
-61
lines changed

3 files changed

+110
-61
lines changed

‎swift/ql/lib/change-notes/2023-10-24-string.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
5+
* Added flow models for `String` methods involving closures such as `String.withUTF8(_:)`.

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,10 @@ private class StringSummaries extends SummaryModelCsv {
6868
";StringProtocol;true;trimmingCharacters(in:);;;Argument[-1];ReturnValue;taint",
6969
";StringProtocol;true;uppercased();;;Argument[-1];ReturnValue;taint",
7070
";StringProtocol;true;uppercased(with:);;;Argument[-1];ReturnValue;taint",
71+
";StringProtocol;true;withCString(_:);;;Argument[-1];Argument[0].Parameter[0].CollectionElement;taint",
72+
";StringProtocol;true;withCString(_:);;;Argument[0].ReturnValue;ReturnValue;value",
73+
";StringProtocol;true;withCString(encodedAs:_:);;;Argument[-1];Argument[1].Parameter[0].CollectionElement;taint",
74+
";StringProtocol;true;withCString(encodedAs:_:);;;Argument[1].ReturnValue;ReturnValue;value",
7175
";String;true;init(decoding:);;;Argument[0];ReturnValue;taint",
7276
";String;true;init(_:);;;Argument[0];ReturnValue;taint",
7377
";String;true;init(_:);;;Argument[0];ReturnValue.OptionalSome;taint",
@@ -110,6 +114,7 @@ private class StringSummaries extends SummaryModelCsv {
110114
";String;true;init(validating:);;;Argument[0];ReturnValue.OptionalSome;taint",
111115
";String;true;init(validatingPlatformString:);;;Argument[0];ReturnValue.OptionalSome;taint",
112116
";String;true;init(validatingPlatformString:);;;Argument[0].CollectionElement;ReturnValue.OptionalSome;taint",
117+
";String;true;init(unsafeUninitializedCapacity:initializingUTF8With:);;;Argument[1].Parameter[0].CollectionElement;ReturnValue;taint",
113118
";String;true;localizedStringWithFormat(_:_:);;;Argument[0];ReturnValue;taint",
114119
";String;true;localizedStringWithFormat(_:_:);;;Argument[1].CollectionElement;ReturnValue;taint",
115120
";String;true;insert(contentsOf:at:);;;Argument[0];Argument[-1];taint",
@@ -125,6 +130,15 @@ private class StringSummaries extends SummaryModelCsv {
125130
";String;true;encode(to:);;;Argument[-1];Argument[0];taint",
126131
";String;true;decodeCString(_:as:repairingInvalidCodeUnits:);;;Argument[0];ReturnValue.TupleElement[0];taint",
127132
";String;true;decodeCString(_:as:repairingInvalidCodeUnits:);;;Argument[0].CollectionElement;ReturnValue.TupleElement[0];taint",
133+
";String;true;withUTF8(_:);;;Argument[-1];Argument[0].Parameter[0].CollectionElement;taint",
134+
";String;true;withUTF8(_:);;;Argument[0].Parameter[0].CollectionElement;Argument[-1];taint",
135+
";String;true;withUTF8(_:);;;Argument[0].ReturnValue;ReturnValue;value",
136+
";String;true;withPlatformString(_:);;;Argument[-1];Argument[0].Parameter[0].CollectionElement;taint",
137+
";String;true;withPlatformString(_:);;;Argument[0].ReturnValue;ReturnValue;value",
138+
";String;true;withMutableCharacters(_:);;;Argument[-1];Argument[0].Parameter[0];value",
139+
";String;true;withMutableCharacters(_:);;;Argument[0].Parameter[0];Argument[-1];value",
140+
";String;true;withMutableCharacters(_:);;;Argument[0].Parameter[0].CollectionElement;Argument[-1];taint",
141+
";String;true;withMutableCharacters(_:);;;Argument[0].ReturnValue;ReturnValue;value",
128142
";LosslessStringConvertible;true;init(_:);;;Argument[0];ReturnValue;taint",
129143
";Substring;true;withUTF8(_:);;;Argument[-1];Argument[0].Parameter[0].CollectionElement;taint",
130144
";Substring;true;withUTF8(_:);;;Argument[0].Parameter[0].CollectionElement;Argument[-1];taint",

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

Lines changed: 91 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ extension String : CVarArg {
7979
init(platformString: UnsafePointer<CInterop.PlatformChar>) { self.init() }
8080

8181
func withPlatformString<Result>(_ body: (UnsafePointer<CInterop.PlatformChar>) throws -> Result) rethrows -> Result { return 0 as! Result }
82-
82+
mutating func withMutableCharacters<R>(_ body: (inout String) -> R) -> R { return 0 as! R }
8383

8484

8585
mutating func replaceSubrange<C>(_ subrange: Range<String.Index>, with newElements: C)
@@ -372,7 +372,7 @@ func taintThroughEncodings() {
372372
})
373373
tainted.withUTF8({
374374
buffer in
375-
sink(arg: buffer[0]) // $ MISSING: tainted=366
375+
sink(arg: buffer[0]) // $ tainted=366
376376
sink(arg: buffer.baseAddress!) // $ MISSING: tainted=366
377377
})
378378

@@ -382,15 +382,15 @@ func taintThroughEncodings() {
382382
})
383383
tainted.withCString({
384384
ptr in
385-
sink(arg: ptr[0]) // $ MISSING: tainted=366
385+
sink(arg: ptr[0]) // $ tainted=366
386386
})
387387
clean.withCString(encodedAs: UTF8.self, {
388388
ptr in
389389
sink(arg: ptr[0])
390390
})
391391
tainted.withCString(encodedAs: UTF8.self, {
392392
ptr in
393-
sink(arg: ptr[0]) // $ MISSING: tainted=366
393+
sink(arg: ptr[0]) // $ tainted=366
394394
})
395395

396396
let arrayString1 = clean.cString(using: String.Encoding.utf8)!
@@ -421,8 +421,8 @@ func taintThroughEncodings() {
421421
})
422422
tainted.withPlatformString({
423423
ptr in
424-
sink(arg: ptr[0]) // $ MISSING: tainted=366
425-
sink(arg: String(platformString: ptr)) // $ MISSING: tainted=366
424+
sink(arg: ptr[0]) // $ tainted=366
425+
sink(arg: String(platformString: ptr)) // $ tainted=366
426426

427427
let buffer = UnsafeBufferPointer(start: ptr, count: 10)
428428
let arrayString = Array(buffer)
@@ -449,26 +449,38 @@ func taintFromUInt8Array() {
449449
var cleanUInt8Values: [UInt8] = [0x41, 0x42, 0x43, 0] // "ABC"
450450
var taintedUInt8Values: [UInt8] = [source4()]
451451

452-
sink(arg: String(unsafeUninitializedCapacity: 256, initializingUTF8With: {
452+
sink(arg: taintedUInt8Values[0]) // $ tainted=450
453+
let r1 = String(unsafeUninitializedCapacity: 256, initializingUTF8With: {
453454
(buffer: UnsafeMutableBufferPointer<UInt8>) -> Int in
454455
sink(arg: buffer[0])
455456
let _ = buffer.initialize(from: cleanUInt8Values)
456457
sink(arg: buffer[0])
457458
return 3
458459
}
459-
))
460-
sink(arg: String(unsafeUninitializedCapacity: 256, initializingUTF8With: { // $ MISSING: tainted=450
460+
)
461+
sink(arg: r1)
462+
let r2 = String(unsafeUninitializedCapacity: 256, initializingUTF8With: {
461463
(buffer: UnsafeMutableBufferPointer<UInt8>) -> Int in
462464
sink(arg: buffer[0])
465+
sink(arg: taintedUInt8Values[0]) // $ MISSING: tainted=450
463466
let _ = buffer.initialize(from: taintedUInt8Values)
464467
sink(arg: buffer[0]) // $ MISSING: tainted=450
465468
return 256
466469
}
467-
))
470+
)
471+
sink(arg: r2) // $ MISSING: tainted=450
472+
let r3 = String(unsafeUninitializedCapacity: 256, initializingUTF8With: {
473+
(buffer: UnsafeMutableBufferPointer<UInt8>) -> Int in
474+
sink(arg: buffer[0])
475+
buffer.update(repeating: source4())
476+
sink(arg: buffer[0]) // $ tainted=475
477+
return 256
478+
}
479+
)
480+
sink(arg: r3) // $ tainted=475
468481

469482
sink(arg: String(bytes: cleanUInt8Values, encoding: String.Encoding.utf8)!)
470483
sink(arg: String(bytes: taintedUInt8Values, encoding: String.Encoding.utf8)!) // $ tainted=450
471-
472484
sink(arg: String(cString: cleanUInt8Values))
473485
sink(arg: String(cString: taintedUInt8Values)) // $ tainted=450
474486

@@ -484,7 +496,6 @@ func taintFromUInt8Array() {
484496
sink(arg: buffer.baseAddress!) // $ MISSING: tainted=450
485497
sink(arg: String(cString: buffer.baseAddress!)) // $ MISSING: tainted=450
486498
})
487-
488499
try! cleanUInt8Values.withUnsafeMutableBytes({
489500
(buffer: UnsafeMutableRawBufferPointer) throws in
490501
sink(arg: buffer[0])
@@ -515,15 +526,15 @@ func taintThroughCCharArray() {
515526
})
516527
taintedCCharValues.withUnsafeBufferPointer({
517528
ptr in
518-
sink(arg: ptr[0]) // $ tainted=506
519-
sink(arg: ptr.baseAddress!) // $ MISSING: tainted=506
520-
sink(arg: String(utf8String: ptr.baseAddress!)!) // $ MISSING: tainted=506
521-
sink(arg: String(validatingUTF8: ptr.baseAddress!)!) // $ MISSING: tainted=506
522-
sink(arg: String(cString: ptr.baseAddress!)) // $ MISSING: tainted=506
529+
sink(arg: ptr[0]) // $ tainted=517
530+
sink(arg: ptr.baseAddress!) // $ MISSING: tainted=517
531+
sink(arg: String(utf8String: ptr.baseAddress!)!) // $ MISSING: tainted=517
532+
sink(arg: String(validatingUTF8: ptr.baseAddress!)!) // $ MISSING: tainted=517
533+
sink(arg: String(cString: ptr.baseAddress!)) // $ MISSING: tainted=517
523534
})
524535

525536
sink(arg: String(cString: cleanCCharValues))
526-
sink(arg: String(cString: taintedCCharValues)) // $ tainted=506
537+
sink(arg: String(cString: taintedCCharValues)) // $ tainted=517
527538
}
528539

529540
func source6() -> unichar { return 0 }
@@ -541,10 +552,10 @@ func taintThroughUnicharArray() {
541552
})
542553
taintedUnicharValues.withUnsafeBufferPointer({
543554
ptr in
544-
sink(arg: ptr[0]) // $ tainted=533
545-
sink(arg: ptr.baseAddress!) // $ MISSING: tainted=533
546-
sink(arg: String(utf16CodeUnits: ptr.baseAddress!, count: ptr.count)) // $ MISSING: tainted=533
547-
sink(arg: String(utf16CodeUnitsNoCopy: ptr.baseAddress!, count: ptr.count, freeWhenDone: false)) // $ MISSING: tainted=533
555+
sink(arg: ptr[0]) // $ tainted=544
556+
sink(arg: ptr.baseAddress!) // $ MISSING: tainted=544
557+
sink(arg: String(utf16CodeUnits: ptr.baseAddress!, count: ptr.count)) // $ MISSING: tainted=5344
558+
sink(arg: String(utf16CodeUnitsNoCopy: ptr.baseAddress!, count: ptr.count, freeWhenDone: false)) // $ MISSING: tainted=544
548559
})
549560
}
550561

@@ -553,43 +564,45 @@ func source7() -> Substring { return Substring() }
553564
func taintThroughSubstring() {
554565
let tainted = source2()
555566

556-
sink(arg: source7()) // $ tainted=556
567+
sink(arg: source7()) // $ tainted=567
557568

558569
let sub1 = tainted[tainted.startIndex ..< tainted.endIndex]
559-
sink(arg: sub1) // $ tainted=554
560-
sink(arg: String(sub1)) // $ tainted=554
570+
sink(arg: sub1) // $ tainted=565
571+
sink(arg: String(sub1)) // $ tainted=565
561572

562573
let sub2 = tainted.prefix(10)
563-
sink(arg: sub2) // $ tainted=554
564-
sink(arg: String(sub2)) // $ tainted=554
574+
sink(arg: sub2) // $ tainted=565
575+
sink(arg: String(sub2)) // $ tainted=565
565576

566577
let sub3 = tainted.prefix(through: tainted.endIndex)
567-
sink(arg: sub3) // $ tainted=554
568-
sink(arg: String(sub3)) // $ tainted=554
578+
sink(arg: sub3) // $ tainted=565
579+
sink(arg: String(sub3)) // $ tainted=565
569580

570581
let sub4 = tainted.prefix(upTo: tainted.endIndex)
571-
sink(arg: sub4) // $ tainted=554
572-
sink(arg: String(sub4)) // $ tainted=554
582+
sink(arg: sub4) // $ tainted=565
583+
sink(arg: String(sub4)) // $ tainted=565
573584

574585
let sub5 = tainted.suffix(10)
575-
sink(arg: sub5) // $ tainted=554
576-
sink(arg: String(sub5)) // $ tainted=554
586+
sink(arg: sub5) // $ tainted=565
587+
sink(arg: String(sub5)) // $ tainted=565
577588

578589
let sub6 = tainted.suffix(from: tainted.startIndex)
579-
sink(arg: sub6) // $ tainted=554
580-
sink(arg: String(sub6)) // $ tainted=554
590+
sink(arg: sub6) // $ tainted=565
591+
sink(arg: String(sub6)) // $ tainted=565
581592
}
582593

583594
func taintedThroughConversion() {
584595
sink(arg: String(0))
585-
sink(arg: String(source())) // $ tainted=585
596+
sink(arg: String(source())) // $ tainted=596
597+
586598
sink(arg: Int(0).description)
587-
sink(arg: source().description) // $ tainted=587
599+
sink(arg: source().description) // $ tainted=599
600+
588601
sink(arg: String(describing: 0))
589-
sink(arg: String(describing: source())) // $ tainted=589
602+
sink(arg: String(describing: source())) // $ tainted=602
590603

591604
sink(arg: Int("123")!)
592-
sink(arg: Int(source2())!) // $ tainted=592
605+
sink(arg: Int(source2())!) // $ tainted=605
593606
}
594607

595608
func untaintedFields() {
@@ -607,7 +620,7 @@ func callbackWithCleanPointer(ptr: UnsafeBufferPointer<String.Element>) throws -
607620
}
608621

609622
func callbackWithTaintedPointer(ptr: UnsafeBufferPointer<String.Element>) throws -> Int {
610-
sink(arg: ptr[0]) // $ tainted=617
623+
sink(arg: ptr[0]) // $ tainted=630
611624

612625
return source()
613626
}
@@ -626,7 +639,7 @@ func furtherTaintThroughCallbacks() {
626639
ptr in
627640
return source()
628641
})
629-
sink(arg: result2!) // $ tainted=627
642+
sink(arg: result2!) // $ tainted=640
630643

631644
// return values from the closure (2)
632645
if let result3 = clean.withContiguousStorageIfAvailable({
@@ -639,28 +652,28 @@ func furtherTaintThroughCallbacks() {
639652
ptr in
640653
return source()
641654
}) {
642-
sink(arg: result4) // $ tainted=640
655+
sink(arg: result4) // $ tainted=653
643656
}
644657

645658
// using a non-closure function
646659
let result5 = try? clean.withContiguousStorageIfAvailable(callbackWithCleanPointer)
647660
sink(arg: result5!)
648661
let result6 = try? tainted.withContiguousStorageIfAvailable(callbackWithTaintedPointer)
649-
sink(arg: result6!) // $ tainted=612
662+
sink(arg: result6!) // $ tainted=625
650663
}
651664

652665
func testAppendingFormat() {
653666
var s1 = source2()
654-
sink(arg: s1.appendingFormat("%s %i", "", 0)) // $ tainted=653
667+
sink(arg: s1.appendingFormat("%s %i", "", 0)) // $ tainted=666
655668

656669
var s2 = ""
657-
sink(arg: s2.appendingFormat(source2(), "", 0)) // $ tainted=657
670+
sink(arg: s2.appendingFormat(source2(), "", 0)) // $ tainted=670
658671

659672
var s3 = ""
660-
sink(arg: s3.appendingFormat("%s %i", source2(), 0)) // $ tainted=660
673+
sink(arg: s3.appendingFormat("%s %i", source2(), 0)) // $ tainted=673
661674

662675
var s4 = ""
663-
sink(arg: s4.appendingFormat("%s %i", "", source())) // $ tainted=663
676+
sink(arg: s4.appendingFormat("%s %i", "", source())) // $ tainted=676
664677
}
665678

666679
func sourceUInt8() -> UInt8 { return 0 }
@@ -669,22 +682,22 @@ func testDecodeCString() {
669682
var input : [UInt8] = [1, 2, 3, sourceUInt8()]
670683

671684
let (str1, repaired1) = String.decodeCString(input, as: UTF8.self)!
672-
sink(arg: str1) // $ tainted=669
685+
sink(arg: str1) // $ tainted=682
673686
sink(arg: repaired1)
674687

675688
input.withUnsafeBufferPointer({
676689
ptr in
677690
let (str2, repaired2) = String.decodeCString(ptr.baseAddress, as: UTF8.self)!
678-
sink(arg: str2) // $ MISSING: tainted=669
691+
sink(arg: str2) // $ MISSING: tainted=682
679692
sink(arg: repaired2)
680693
})
681694

682695
let (str3, repaired3) = String.decodeCString(source2(), as: UTF8.self)!
683-
sink(arg: str3) // $ tainted=682
696+
sink(arg: str3) // $ tainted=695
684697
sink(arg: repaired3)
685698

686699
let (str4, repaired4) = String.decodeCString(&input, as: UTF8.self)!
687-
sink(arg: str4) // $ tainted=669
700+
sink(arg: str4) // $ tainted=682
688701
sink(arg: repaired4)
689702
}
690703

@@ -693,26 +706,43 @@ func testSubstringMembers() {
693706
let tainted = source2()
694707

695708
let sub1 = tainted[..<tainted.index(tainted.endIndex, offsetBy: -5)]
696-
sink(arg: sub1) // $ tainted=693
697-
sink(arg: sub1.base) // $ tainted=693
698-
sink(arg: sub1.utf8) // $ tainted=693
699-
sink(arg: sub1.capitalized) // $ tainted=693
700-
sink(arg: sub1.description) // $ tainted=693
709+
sink(arg: sub1) // $ tainted=706
710+
sink(arg: sub1.base) // $ tainted=706
711+
sink(arg: sub1.utf8) // $ tainted=706
712+
sink(arg: sub1.capitalized) // $ tainted=706
713+
sink(arg: sub1.description) // $ tainted=706
701714

702715
var sub2 = tainted[tainted.index(tainted.startIndex, offsetBy: 5)...]
703-
sink(arg: sub2) // $ tainted=693
716+
sink(arg: sub2) // $ tainted=706
704717
let result1 = sub2.withUTF8({
705718
buffer in
706-
sink(arg: buffer[0]) // $ tainted=693
719+
sink(arg: buffer[0]) // $ tainted=706
707720
return source()
708721
})
709-
sink(arg: result1) // $ tainted=707
722+
sink(arg: result1) // $ tainted=720
710723

711724
let sub3 = Substring(sub2.utf8)
712-
sink(arg: sub3) // $ tainted=693
725+
sink(arg: sub3) // $ tainted=706
713726

714727
var sub4 = clean.prefix(10)
715728
sink(arg: sub4)
716729
sub4.replaceSubrange(..<clean.endIndex, with: sub1)
717-
sink(arg: sub4) // $ tainted=693
730+
sink(arg: sub4) // $ tainted=706
731+
}
732+
733+
// ---
734+
735+
func taintMutableCharacters() {
736+
var str = ""
737+
738+
sink(arg: str)
739+
let rtn = str.withMutableCharacters({
740+
chars in
741+
sink(arg: chars)
742+
chars.append(source2())
743+
sink(arg: chars) // $ tainted=742
744+
return source()
745+
})
746+
sink(arg: rtn) // $ tainted=744
747+
sink(arg: str) // $ tainted=742
718748
}

0 commit comments

Comments
 (0)