diff --git a/swift/ql/lib/change-notes/2023-09-29-nsstring.md b/swift/ql/lib/change-notes/2023-09-29-nsstring.md new file mode 100644 index 000000000000..b1c806532268 --- /dev/null +++ b/swift/ql/lib/change-notes/2023-09-29-nsstring.md @@ -0,0 +1,5 @@ +--- +category: minorAnalysis +--- + +* Modelled varargs function in `NSString` more accurately. diff --git a/swift/ql/lib/codeql/swift/frameworks/StandardLibrary/NsString.qll b/swift/ql/lib/codeql/swift/frameworks/StandardLibrary/NsString.qll index f866ba23a176..2044c7757a26 100644 --- a/swift/ql/lib/codeql/swift/frameworks/StandardLibrary/NsString.qll +++ b/swift/ql/lib/codeql/swift/frameworks/StandardLibrary/NsString.qll @@ -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", @@ -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", @@ -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", @@ -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", ] } } diff --git a/swift/ql/test/library-tests/dataflow/taint/libraries/TaintInline.expected b/swift/ql/test/library-tests/dataflow/taint/libraries/TaintInline.expected index 48de9172b362..8ec8033d086e 100644 --- a/swift/ql/test/library-tests/dataflow/taint/libraries/TaintInline.expected +++ b/swift/ql/test/library-tests/dataflow/taint/libraries/TaintInline.expected @@ -1,2 +1,2 @@ -failures testFailures +failures diff --git a/swift/ql/test/library-tests/dataflow/taint/libraries/nsstring.swift b/swift/ql/test/library-tests/dataflow/taint/libraries/nsstring.swift index 8f961b35d17f..3be4e4d12645 100644 --- a/swift/ql/test/library-tests/dataflow/taint/libraries/nsstring.swift +++ b/swift/ql/test/library-tests/dataflow/taint/libraries/nsstring.swift @@ -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) -> Any? { return nil } class func string(withCString bytes: UnsafePointer, length: Int) -> Any? { return nil } @@ -185,7 +185,7 @@ func sourceUnsafeMutableRawPointer() -> UnsafeMutableRawPointer { return (nil as func sourceCString() -> UnsafePointer { return (nil as UnsafePointer?)! } func sourceData() -> Data { return Data(0) } func sourceStringArray() -> [String] { return [] } - +func sourceInt() -> Int { return 0 } func sink(arg: Any) {} func taintThroughInterpolatedStrings() { @@ -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 diff --git a/swift/ql/test/library-tests/dataflow/taint/libraries/string.swift b/swift/ql/test/library-tests/dataflow/taint/libraries/string.swift index 2544512646f9..653fbeef61a8 100644 --- a/swift/ql/test/library-tests/dataflow/taint/libraries/string.swift +++ b/swift/ql/test/library-tests/dataflow/taint/libraries/string.swift @@ -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 @@ -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