Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Swift: Improve taint models for NSString #14266

Merged
merged 3 commits into from
Sep 21, 2023
Merged

Swift: Improve taint models for NSString #14266

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f8c5a9a
Swift: Test localizedStringWithFormat a bit better.
geoffw0 Sep 19, 2023
1b74b49
Swift: Improve NSString models for varargs functions.
geoffw0 Sep 19, 2023
af315c5
Swift: Change note.
geoffw0 Sep 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions swift/ql/lib/change-notes/2023-09-29-nsstring.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
category: minorAnalysis
---

* Modelled varargs function in `NSString` more accurately.
15 changes: 10 additions & 5 deletions swift/ql/lib/codeql/swift/frameworks/StandardLibrary/NsString.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,10 @@ private class NsStringSummaries extends SummaryModelCsv {
";NSString;true;init(format:arguments:);;;Argument[0..1];ReturnValue;taint",
";NSString;true;init(format:locale:arguments:);;;Argument[0];ReturnValue;taint",
";NSString;true;init(format:locale:arguments:);;;Argument[2];ReturnValue;taint",
";NSString;true;init(format:_:);;;Argument[0];ReturnValue;taint", //0..
";NSString;true;init(format:locale:_:);;;Argument[0];ReturnValue;taint", //0,2..
";NSString;true;init(format:_:);;;Argument[0];ReturnValue;taint",
";NSString;true;init(format:_:);;;Argument[1].CollectionElement;ReturnValue;taint",
";NSString;true;init(format:locale:_:);;;Argument[0];ReturnValue;taint",
";NSString;true;init(format:locale:_:);;;Argument[2].CollectionElement;ReturnValue;taint",
";NSString;true;init(data:encoding:);;;Argument[0];ReturnValue;taint",
";NSString;true;init(contentsOfFile:);;;Argument[0];ReturnValue;taint",
";NSString;true;init(contentsOfFile:encoding:);;;Argument[0];ReturnValue;taint",
Expand All @@ -60,7 +62,8 @@ private class NsStringSummaries extends SummaryModelCsv {
";NSString;true;init(contentsOf:encoding:);;;Argument[0];ReturnValue;taint",
";NSString;true;init(contentsOf:usedEncoding:);;;Argument[0];ReturnValue;taint",
";NSString;true;init(coder:);;;Argument[0];ReturnValue;taint",
";NSString;true;localizedStringWithFormat(_:_:);;;Argument[0];ReturnValue;taint", //0..
";NSString;true;localizedStringWithFormat(_:_:);;;Argument[0];ReturnValue;taint",
";NSString;true;localizedStringWithFormat(_:_:);;;Argument[1].CollectionElement;ReturnValue;taint",
";NSString;true;character(at:);;;Argument[-1];ReturnValue;taint",
";NSString;true;getCharacters(_:);;;Argument[-1];Argument[0];taint",
";NSString;true;getCharacters(_:range:);;;Argument[-1];Argument[0];taint",
Expand All @@ -72,7 +75,8 @@ private class NsStringSummaries extends SummaryModelCsv {
";NSString;true;getCString(_:maxLength:);;;Argument[-1];Argument[0];taint",
";NSString;true;getCString(_:maxLength:encoding:);;;Argument[-1];Argument[0];taint",
";NSString;true;getCString(_:maxLength:range:remaining:);;;Argument[-1];Argument[0];taint",
";NSString;true;appendingFormat(_:_:);;;Argument[-1..0];ReturnValue;taint", // -1..
";NSString;true;appendingFormat(_:_:);;;Argument[-1..0];ReturnValue;taint",
";NSString;true;appendingFormat(_:_:);;;Argument[1].CollectionElement;ReturnValue;taint",
";NSString;true;appending(_:);;;Argument[-1..0];ReturnValue;taint",
";NSString;true;padding(toLength:withPad:startingAt:);;;Argument[-1];ReturnValue;taint",
";NSString;true;padding(toLength:withPad:startingAt:);;;Argument[1];ReturnValue;taint",
Expand Down Expand Up @@ -119,7 +123,8 @@ private class NsStringSummaries extends SummaryModelCsv {
";NSMutableString;true;replaceCharacters(in:with:);;;Argument[1];Argument[-1];taint",
";NSMutableString;true;replaceOccurrences(of:with:options:range:);;;Argument[1];Argument[-1];taint",
";NSMutableString;true;setString(_:);;;Argument[0];Argument[-1];taint",
";NSMutableString;true;appendFormat(_:_:);;;Argument[0];Argument[-1];taint", //0..
";NSMutableString;true;appendFormat(_:_:);;;Argument[0];Argument[-1];taint",
";NSMutableString;true;appendFormat(_:_:);;;Argument[1].CollectionElement;Argument[-1];taint",
]
}
}
Expand Down
2 changes: 1 addition & 1 deletion swift/ql/test/library-tests/dataflow/taint/libraries/TaintInline.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
failures
testFailures
failures
8 changes: 4 additions & 4 deletions swift/ql/test/library-tests/dataflow/taint/libraries/nsstring.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ class NSString : NSObject, NSCopying, NSMutableCopying {
func copy(with zone: NSZone? = nil) -> Any { return 0 }
func mutableCopy(with zone: NSZone? = nil) -> Any { return 0 }

class func localizedStringWithFormat(_ format: NSString, _ args: CVarArg) -> Self { return (nil as Self?)! }
class func localizedStringWithFormat(_ format: NSString, _ args: CVarArg...) -> Self { return (nil as Self?)! }
class func path(withComponents components: [String]) -> String { return "" }
class func string(withCString bytes: UnsafePointer<CChar>) -> Any? { return nil }
class func string(withCString bytes: UnsafePointer<CChar>, length: Int) -> Any? { return nil }
Expand Down Expand Up @@ -185,7 +185,7 @@ func sourceUnsafeMutableRawPointer() -> UnsafeMutableRawPointer { return (nil as
func sourceCString() -> UnsafePointer<CChar> { return (nil as UnsafePointer<CChar>?)! }
func sourceData() -> Data { return Data(0) }
func sourceStringArray() -> [String] { return [] }

func sourceInt() -> Int { return 0 }
func sink(arg: Any) {}

func taintThroughInterpolatedStrings() {
Expand Down Expand Up @@ -244,8 +244,8 @@ func taintThroughInterpolatedStrings() {

let harmless = NSString(string: "harmless")
let myRange = NSRange(location:0, length: 128)

sink(arg: NSString.localizedStringWithFormat(sourceNSString(), (nil as CVarArg?)!)) // $ tainted=248
sink(arg: NSString.localizedStringWithFormat(NSString(string: "%i %i %i"), 1, sourceInt(), 3)) // $ tainted=247
sink(arg: NSString.localizedStringWithFormat(sourceNSString(), 1, 2, 3)) // $ tainted=248
sink(arg: sourceNSString().character(at: 0)) // $ tainted=249
sink(arg: sourceNSString().cString(using: 0)!) // $ tainted=250
sink(arg: sourceNSString().cString()) // $ tainted=251
Expand Down
2 changes: 1 addition & 1 deletion swift/ql/test/library-tests/dataflow/taint/libraries/string.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,7 @@ func taintThroughSimpleStringOperations() {
sink(arg: String(format: tainted, locale: nil, 1, 2, 3)) // $ tainted=217
sink(arg: String(format: tainted, locale: nil, arguments: [])) // $ tainted=217
sink(arg: String.localizedStringWithFormat(tainted, 1, 2, 3)) // $ tainted=217
sink(arg: String.localizedStringWithFormat("%i %s %i", 1, tainted, 3)) // $ tainted=217
sink(arg: String(format: "%s", tainted)) // $ tainted=217
sink(arg: String(format: "%i %i %i", 1, 2, taintedInt)) // $ tainted=218

Expand All @@ -235,7 +236,6 @@ func taintThroughSimpleStringOperations() {
sink(arg: tainted.dropFirst(10)) // $ tainted=217
sink(arg: tainted.dropLast(10)) // $ tainted=217
sink(arg: tainted.substring(from: tainted.startIndex)) // $ tainted=217

sink(arg: tainted.lowercased()) // $ tainted=217
sink(arg: tainted.uppercased()) // $ tainted=217
sink(arg: tainted.lowercased(with: nil)) // $ tainted=217
Expand Down