From 9ec2f9204b9c11dcfb6477481563c1dc65dd781e Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Thu, 2 Nov 2023 10:04:28 +0000 Subject: [PATCH 1/6] Swift: Correct components(separatedBy:) models. --- swift/ql/lib/codeql/swift/frameworks/StandardLibrary/String.qll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/swift/ql/lib/codeql/swift/frameworks/StandardLibrary/String.qll b/swift/ql/lib/codeql/swift/frameworks/StandardLibrary/String.qll index 44c90da5839..316e60c21ae 100644 --- a/swift/ql/lib/codeql/swift/frameworks/StandardLibrary/String.qll +++ b/swift/ql/lib/codeql/swift/frameworks/StandardLibrary/String.qll @@ -47,7 +47,7 @@ private class StringSummaries extends SummaryModelCsv { ";StringProtocol;true;capitalized(with:);;;Argument[-1];ReturnValue;taint", ";StringProtocol;true;completePath(into:caseSensitive:matchesInto:filterTypes:);;;Argument[-1];Argument[0].OptionalSome.CollectionElement;taint", ";StringProtocol;true;completePath(into:caseSensitive:matchesInto:filterTypes:);;;Argument[-1];Argument[2].OptionalSome.CollectionElement.CollectionElement;taint", - ";StringProtocol;true;components(separatedBy:);;;Argument[-1];ReturnValue;taint", + ";StringProtocol;true;components(separatedBy:);;;Argument[-1];ReturnValue.CollectionElement;taint", ";StringProtocol;true;data(using:allowLossyConversion:);;;Argument[-1];ReturnValue;taint", ";StringProtocol;true;folding(options:locale:);;;Argument[-1];ReturnValue;taint", ";StringProtocol;true;getBytes(_:maxLength:usedLength:encoding:options:range:remaining:);;;Argument[-1];Argument[0].CollectionElement;taint", From 892beeab6d4326fafeb60de2d1aea693b73b1e61 Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Mon, 13 Nov 2023 18:11:39 +0000 Subject: [PATCH 2/6] Swift: Add test case. --- .../dataflow/taint/libraries/webview.swift | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/swift/ql/test/library-tests/dataflow/taint/libraries/webview.swift b/swift/ql/test/library-tests/dataflow/taint/libraries/webview.swift index 43b291163d6..0bc2614aaf6 100644 --- a/swift/ql/test/library-tests/dataflow/taint/libraries/webview.swift +++ b/swift/ql/test/library-tests/dataflow/taint/libraries/webview.swift @@ -76,7 +76,7 @@ struct URLRequest {} // --- tests --- -func source() -> Any { return "" } +func source(_ label: String? = "") -> Any { return "" } func sink(_: Any) {} func testInheritBodyTaint() { @@ -146,6 +146,9 @@ func testWKUserScript() { } func testWKNavigationAction() { - let src = source() as! WKNavigationAction - sink(src.request) // $ tainted=149 -} \ No newline at end of file + let src = source("WKNavigationAction") as! WKNavigationAction + sink(src.request) // $ tainted=WKNavigationAction + + let keypath = \WKNavigationAction.request + sink(src[keyPath: keypath]) // $ MISSING: tainted=WKNavigationAction +} From 463096e4bea4f855c54d2dc26e4f25ab8a612596 Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Thu, 2 Nov 2023 09:27:33 +0000 Subject: [PATCH 3/6] Swift: Modernize tainted content in WebView.qll. --- .../frameworks/StandardLibrary/WebView.qll | 20 ++++++++----------- .../dataflow/taint/libraries/webview.swift | 2 +- 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/swift/ql/lib/codeql/swift/frameworks/StandardLibrary/WebView.qll b/swift/ql/lib/codeql/swift/frameworks/StandardLibrary/WebView.qll index da54683029f..876e8623fd6 100644 --- a/swift/ql/lib/codeql/swift/frameworks/StandardLibrary/WebView.qll +++ b/swift/ql/lib/codeql/swift/frameworks/StandardLibrary/WebView.qll @@ -74,19 +74,15 @@ private class WKNavigationDelegateSource extends RemoteFlowSource { } /** - * A taint step implying that, if a `WKNavigationAction` is tainted, its `request` field is also tainted. + * A content implying that, if a `WKNavigationAction` is tainted, its + * `request` field is also tainted. */ -private class WKNavigationActionTaintStep extends AdditionalTaintStep { - override predicate step(DataFlow::Node n1, DataFlow::Node n2) { - exists(MemberRefExpr e, Expr self, VarDecl member | - self.getType().getName() = "WKNavigationAction" and - member.getName() = "request" - | - e.getBase() = self and - e.getMember() = member and - n1.asExpr() = self and - n2.asExpr() = e - ) +private class UrlRequestFieldsInheritTaint extends TaintInheritingContent, + DataFlow::Content::FieldContent +{ + UrlRequestFieldsInheritTaint() { + this.getField().getEnclosingDecl().asNominalTypeDecl().getName() = "WKNavigationAction" and + this.getField().getName() = "request" } } diff --git a/swift/ql/test/library-tests/dataflow/taint/libraries/webview.swift b/swift/ql/test/library-tests/dataflow/taint/libraries/webview.swift index 0bc2614aaf6..92fb7d12498 100644 --- a/swift/ql/test/library-tests/dataflow/taint/libraries/webview.swift +++ b/swift/ql/test/library-tests/dataflow/taint/libraries/webview.swift @@ -150,5 +150,5 @@ func testWKNavigationAction() { sink(src.request) // $ tainted=WKNavigationAction let keypath = \WKNavigationAction.request - sink(src[keyPath: keypath]) // $ MISSING: tainted=WKNavigationAction + sink(src[keyPath: keypath]) // $ tainted=WKNavigationAction } From 985d1990eb3a6929faa17dae0c7a842e1c348f0d Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Thu, 2 Nov 2023 11:49:05 +0000 Subject: [PATCH 4/6] Swift: Fix typo. --- .../ql/lib/codeql/swift/frameworks/StandardLibrary/WebView.qll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/swift/ql/lib/codeql/swift/frameworks/StandardLibrary/WebView.qll b/swift/ql/lib/codeql/swift/frameworks/StandardLibrary/WebView.qll index da54683029f..c12c678cc6c 100644 --- a/swift/ql/lib/codeql/swift/frameworks/StandardLibrary/WebView.qll +++ b/swift/ql/lib/codeql/swift/frameworks/StandardLibrary/WebView.qll @@ -39,7 +39,7 @@ private class WKScriptMessageBodyInheritsTaint extends TaintInheritingContent, } /** - * A type or extension delcaration that adopts the protocol `WKNavigationDelegate`. + * A type or extension declaration that adopts the protocol `WKNavigationDelegate`. */ private class AdoptsWkNavigationDelegate extends Decl { AdoptsWkNavigationDelegate() { From b157d73c101bc48b3a84cee127dae440a43d2357 Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Thu, 2 Nov 2023 11:30:54 +0000 Subject: [PATCH 5/6] Swift: Make the URLRequest test more accurate. --- .../dataflow/taint/libraries/url.swift | 28 ++++++++++--------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/swift/ql/test/library-tests/dataflow/taint/libraries/url.swift b/swift/ql/test/library-tests/dataflow/taint/libraries/url.swift index 799eaa85807..15113886e38 100644 --- a/swift/ql/test/library-tests/dataflow/taint/libraries/url.swift +++ b/swift/ql/test/library-tests/dataflow/taint/libraries/url.swift @@ -159,11 +159,12 @@ struct URLRequest : CustomStringConvertible, CustomDebugStringConvertible { enum NetworkServiceType { case none } enum Attribution { case none } var cachePolicy: CachePolicy = .none - var httpMethod: String = "" - var url: URL = URL(string: "")! - var httpBody: Data = Data("") + var httpMethod: String? = "" + var url: URL? = URL(string: "") + var httpBody: Data? = Data("") var httpBodyStream: InputStream? = nil var mainDocument: URL = URL(string: "")! + var mainDocumentURL: URL? = URL(string: "") var allHTTPHeaderFields: [String : String]? = nil var timeoutInterval: TimeInterval = TimeInterval() var httpShouldHandleCookies: Bool = false @@ -204,7 +205,6 @@ func sink(data: Data) {} func sink(string: String) {} func sink(int: Int) {} func sink(any: Any) {} - func taintThroughURL() { let clean = "http://example.com/" let tainted = source() as! String @@ -436,14 +436,16 @@ func taintThroughUrlRequest() { sink(any: tainted.cachePolicy) sink(any: clean.httpMethod) sink(any: tainted.httpMethod) - sink(any: clean.url) - sink(any: tainted.url) // $ tainted=431 - sink(any: clean.httpBody) - sink(any: tainted.httpBody) // $ tainted=431 + sink(any: clean.url!) + sink(any: tainted.url!) // $ tainted=431 + sink(any: clean.httpBody!) + sink(any: tainted.httpBody!) // $ tainted=431 sink(any: clean.httpBodyStream!) sink(any: tainted.httpBodyStream!) // $ tainted=431 sink(any: clean.mainDocument) sink(any: tainted.mainDocument) // $ tainted=431 + sink(any: clean.mainDocumentURL!) + sink(any: tainted.mainDocumentURL!) // $ MISSING: tainted=431 sink(any: clean.allHTTPHeaderFields!) sink(any: tainted.allHTTPHeaderFields!) // $ tainted=431 sink(any: clean.timeoutInterval) @@ -481,19 +483,19 @@ func taintThroughUrlResource() { let tainted = source() as! URLResource sink(string: clean.name) - sink(string: tainted.name) // $ tainted=481 + sink(string: tainted.name) // $ tainted=483 sink(string: clean.subdirectory!) - sink(string: tainted.subdirectory!) // $ tainted=481 + sink(string: tainted.subdirectory!) // $ tainted=483 } func taintUrlAsync() async throws { let tainted = source() as! String let urlTainted = URL(string: tainted)! - sink(any: urlTainted.lines) // $ tainted=490 + sink(any: urlTainted.lines) // $ tainted=492 for try await line in urlTainted.lines { - sink(string: line) // $ MISSING: tainted=490 + sink(string: line) // $ MISSING: tainted=492 } } @@ -510,5 +512,5 @@ func closureReturnValue() { ptr in return source() as! String }) - sink(string: r2) // $ tainted=511 + sink(string: r2) // $ tainted=513 } From 5a451e964dfb2c364ea458a0cad1cb64b64ce8c8 Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Mon, 13 Nov 2023 19:51:51 +0000 Subject: [PATCH 6/6] Swift: Model mainDocumentURL. --- .../lib/codeql/swift/frameworks/StandardLibrary/Url.qll | 8 +++++--- .../test/library-tests/dataflow/taint/libraries/url.swift | 2 +- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/swift/ql/lib/codeql/swift/frameworks/StandardLibrary/Url.qll b/swift/ql/lib/codeql/swift/frameworks/StandardLibrary/Url.qll index a24a394dd0f..07f086a510f 100644 --- a/swift/ql/lib/codeql/swift/frameworks/StandardLibrary/Url.qll +++ b/swift/ql/lib/codeql/swift/frameworks/StandardLibrary/Url.qll @@ -22,8 +22,7 @@ private class UriFieldsInheritTaint extends TaintInheritingContent, DataFlow::Co } /** - * A content implying that, if a `URLRequest` is tainted, then its fields `url`, `httpBody`, - * `httpBodyStream`, `mainDocument` and `allHTTPHeaderFields` are tainted. + * A content implying that, if a `URLRequest` is tainted, then certain fields tainted. */ private class UrlRequestFieldsInheritTaint extends TaintInheritingContent, DataFlow::Content::FieldContent @@ -31,7 +30,10 @@ private class UrlRequestFieldsInheritTaint extends TaintInheritingContent, UrlRequestFieldsInheritTaint() { this.getField().getEnclosingDecl().asNominalTypeDecl().getName() = "URLRequest" and this.getField().getName() = - ["url", "httpBody", "httpBodyStream", "mainDocument", "allHTTPHeaderFields"] + [ + "url", "httpBody", "httpBodyStream", "mainDocument", "mainDocumentURL", + "allHTTPHeaderFields" + ] } } diff --git a/swift/ql/test/library-tests/dataflow/taint/libraries/url.swift b/swift/ql/test/library-tests/dataflow/taint/libraries/url.swift index 15113886e38..b757805380b 100644 --- a/swift/ql/test/library-tests/dataflow/taint/libraries/url.swift +++ b/swift/ql/test/library-tests/dataflow/taint/libraries/url.swift @@ -445,7 +445,7 @@ func taintThroughUrlRequest() { sink(any: clean.mainDocument) sink(any: tainted.mainDocument) // $ tainted=431 sink(any: clean.mainDocumentURL!) - sink(any: tainted.mainDocumentURL!) // $ MISSING: tainted=431 + sink(any: tainted.mainDocumentURL!) // $ tainted=431 sink(any: clean.allHTTPHeaderFields!) sink(any: tainted.allHTTPHeaderFields!) // $ tainted=431 sink(any: clean.timeoutInterval)