From 09b669a58448fafa7bde94c5f4d3ddf034b4d721 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nora=20Dimitrijevi=C4=87?= Date: Tue, 15 Nov 2022 21:57:46 +0100 Subject: [PATCH] Swift: Add direct call to remote source to a test Strangely, there are two separate paths to each of the JSEvaluateScript sinks: one passing through the JSString constructor, one omitting this step. --- .../Security/CWE-094/UnsafeJsEval.expected | 23 ++++++++++++------- .../Security/CWE-094/UnsafeJsEval.swift | 4 ++-- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/swift/ql/test/query-tests/Security/CWE-094/UnsafeJsEval.expected b/swift/ql/test/query-tests/Security/CWE-094/UnsafeJsEval.expected index 05881a11eb5..4e04cad1468 100644 --- a/swift/ql/test/query-tests/Security/CWE-094/UnsafeJsEval.expected +++ b/swift/ql/test/query-tests/Security/CWE-094/UnsafeJsEval.expected @@ -1,17 +1,17 @@ edges | UnsafeJsEval.swift:124:21:124:42 | string : | UnsafeJsEval.swift:124:70:124:70 | string : | | UnsafeJsEval.swift:165:10:165:37 | try ... : | UnsafeJsEval.swift:201:21:201:35 | call to getRemoteData() : | -| UnsafeJsEval.swift:165:10:165:37 | try ... : | UnsafeJsEval.swift:204:7:204:21 | call to getRemoteData() : | | UnsafeJsEval.swift:165:14:165:37 | call to init(contentsOf:) : | UnsafeJsEval.swift:165:10:165:37 | try ... : | | UnsafeJsEval.swift:201:21:201:35 | call to getRemoteData() : | UnsafeJsEval.swift:205:7:205:7 | remoteString : | | UnsafeJsEval.swift:201:21:201:35 | call to getRemoteData() : | UnsafeJsEval.swift:208:7:208:39 | ... .+(_:_:) ... : | | UnsafeJsEval.swift:201:21:201:35 | call to getRemoteData() : | UnsafeJsEval.swift:214:7:214:49 | call to init(decoding:as:) : | -| UnsafeJsEval.swift:204:7:204:21 | call to getRemoteData() : | UnsafeJsEval.swift:265:13:265:13 | string : | -| UnsafeJsEval.swift:204:7:204:21 | call to getRemoteData() : | UnsafeJsEval.swift:268:13:268:13 | string : | -| UnsafeJsEval.swift:204:7:204:21 | call to getRemoteData() : | UnsafeJsEval.swift:276:13:276:13 | string : | -| UnsafeJsEval.swift:204:7:204:21 | call to getRemoteData() : | UnsafeJsEval.swift:279:13:279:13 | string : | -| UnsafeJsEval.swift:204:7:204:21 | call to getRemoteData() : | UnsafeJsEval.swift:285:13:285:13 | string : | -| UnsafeJsEval.swift:204:7:204:21 | call to getRemoteData() : | UnsafeJsEval.swift:299:13:299:13 | string : | +| UnsafeJsEval.swift:204:7:204:66 | try! ... : | UnsafeJsEval.swift:265:13:265:13 | string : | +| UnsafeJsEval.swift:204:7:204:66 | try! ... : | UnsafeJsEval.swift:268:13:268:13 | string : | +| UnsafeJsEval.swift:204:7:204:66 | try! ... : | UnsafeJsEval.swift:276:13:276:13 | string : | +| UnsafeJsEval.swift:204:7:204:66 | try! ... : | UnsafeJsEval.swift:279:13:279:13 | string : | +| UnsafeJsEval.swift:204:7:204:66 | try! ... : | UnsafeJsEval.swift:285:13:285:13 | string : | +| UnsafeJsEval.swift:204:7:204:66 | try! ... : | UnsafeJsEval.swift:299:13:299:13 | string : | +| UnsafeJsEval.swift:204:12:204:66 | call to init(contentsOf:) : | UnsafeJsEval.swift:204:7:204:66 | try! ... : | | UnsafeJsEval.swift:205:7:205:7 | remoteString : | UnsafeJsEval.swift:265:13:265:13 | string : | | UnsafeJsEval.swift:205:7:205:7 | remoteString : | UnsafeJsEval.swift:268:13:268:13 | string : | | UnsafeJsEval.swift:205:7:205:7 | remoteString : | UnsafeJsEval.swift:276:13:276:13 | string : | @@ -56,7 +56,8 @@ nodes | UnsafeJsEval.swift:165:10:165:37 | try ... : | semmle.label | try ... : | | UnsafeJsEval.swift:165:14:165:37 | call to init(contentsOf:) : | semmle.label | call to init(contentsOf:) : | | UnsafeJsEval.swift:201:21:201:35 | call to getRemoteData() : | semmle.label | call to getRemoteData() : | -| UnsafeJsEval.swift:204:7:204:21 | call to getRemoteData() : | semmle.label | call to getRemoteData() : | +| UnsafeJsEval.swift:204:7:204:66 | try! ... : | semmle.label | try! ... : | +| UnsafeJsEval.swift:204:12:204:66 | call to init(contentsOf:) : | semmle.label | call to init(contentsOf:) : | | UnsafeJsEval.swift:205:7:205:7 | remoteString : | semmle.label | remoteString : | | UnsafeJsEval.swift:208:7:208:39 | ... .+(_:_:) ... : | semmle.label | ... .+(_:_:) ... : | | UnsafeJsEval.swift:214:7:214:49 | call to init(decoding:as:) : | semmle.label | call to init(decoding:as:) : | @@ -85,8 +86,14 @@ subpaths | UnsafeJsEval.swift:301:31:301:84 | call to JSStringCreateWithUTF8CString(_:) : | UnsafeJsEval.swift:124:21:124:42 | string : | UnsafeJsEval.swift:124:70:124:70 | string : | UnsafeJsEval.swift:301:16:301:85 | call to JSStringRetain(_:) : | #select | UnsafeJsEval.swift:266:22:266:107 | call to init(source:injectionTime:forMainFrameOnly:) | UnsafeJsEval.swift:165:14:165:37 | call to init(contentsOf:) : | UnsafeJsEval.swift:266:22:266:107 | call to init(source:injectionTime:forMainFrameOnly:) | Evaluation of uncontrolled JavaScript from a remote source. | +| UnsafeJsEval.swift:266:22:266:107 | call to init(source:injectionTime:forMainFrameOnly:) | UnsafeJsEval.swift:204:12:204:66 | call to init(contentsOf:) : | UnsafeJsEval.swift:266:22:266:107 | call to init(source:injectionTime:forMainFrameOnly:) | Evaluation of uncontrolled JavaScript from a remote source. | | UnsafeJsEval.swift:269:22:269:124 | call to init(source:injectionTime:forMainFrameOnly:in:) | UnsafeJsEval.swift:165:14:165:37 | call to init(contentsOf:) : | UnsafeJsEval.swift:269:22:269:124 | call to init(source:injectionTime:forMainFrameOnly:in:) | Evaluation of uncontrolled JavaScript from a remote source. | +| UnsafeJsEval.swift:269:22:269:124 | call to init(source:injectionTime:forMainFrameOnly:in:) | UnsafeJsEval.swift:204:12:204:66 | call to init(contentsOf:) : | UnsafeJsEval.swift:269:22:269:124 | call to init(source:injectionTime:forMainFrameOnly:in:) | Evaluation of uncontrolled JavaScript from a remote source. | | UnsafeJsEval.swift:277:26:277:26 | string | UnsafeJsEval.swift:165:14:165:37 | call to init(contentsOf:) : | UnsafeJsEval.swift:277:26:277:26 | string | Evaluation of uncontrolled JavaScript from a remote source. | +| UnsafeJsEval.swift:277:26:277:26 | string | UnsafeJsEval.swift:204:12:204:66 | call to init(contentsOf:) : | UnsafeJsEval.swift:277:26:277:26 | string | Evaluation of uncontrolled JavaScript from a remote source. | | UnsafeJsEval.swift:280:26:280:26 | string | UnsafeJsEval.swift:165:14:165:37 | call to init(contentsOf:) : | UnsafeJsEval.swift:280:26:280:26 | string | Evaluation of uncontrolled JavaScript from a remote source. | +| UnsafeJsEval.swift:280:26:280:26 | string | UnsafeJsEval.swift:204:12:204:66 | call to init(contentsOf:) : | UnsafeJsEval.swift:280:26:280:26 | string | Evaluation of uncontrolled JavaScript from a remote source. | | UnsafeJsEval.swift:291:17:291:17 | jsstr | UnsafeJsEval.swift:165:14:165:37 | call to init(contentsOf:) : | UnsafeJsEval.swift:291:17:291:17 | jsstr | Evaluation of uncontrolled JavaScript from a remote source. | +| UnsafeJsEval.swift:291:17:291:17 | jsstr | UnsafeJsEval.swift:204:12:204:66 | call to init(contentsOf:) : | UnsafeJsEval.swift:291:17:291:17 | jsstr | Evaluation of uncontrolled JavaScript from a remote source. | | UnsafeJsEval.swift:305:17:305:17 | jsstr | UnsafeJsEval.swift:165:14:165:37 | call to init(contentsOf:) : | UnsafeJsEval.swift:305:17:305:17 | jsstr | Evaluation of uncontrolled JavaScript from a remote source. | +| UnsafeJsEval.swift:305:17:305:17 | jsstr | UnsafeJsEval.swift:204:12:204:66 | call to init(contentsOf:) : | UnsafeJsEval.swift:305:17:305:17 | jsstr | Evaluation of uncontrolled JavaScript from a remote source. | diff --git a/swift/ql/test/query-tests/Security/CWE-094/UnsafeJsEval.swift b/swift/ql/test/query-tests/Security/CWE-094/UnsafeJsEval.swift index e65c182a4c3..69ccaba2f81 100644 --- a/swift/ql/test/query-tests/Security/CWE-094/UnsafeJsEval.swift +++ b/swift/ql/test/query-tests/Security/CWE-094/UnsafeJsEval.swift @@ -175,7 +175,7 @@ func testAsync(_ sink: @escaping (String) async throws -> ()) { let remoteString = getRemoteData() try! await sink(localString) // GOOD: the HTML data is local - try! await sink(getRemoteData()) // BAD [NOT DETECTED - TODO: extract Callables of @MainActor method calls]: HTML contains remote input, may access local secrets + try! await sink(try String(contentsOf: URL(string: "http://example.com/")!)) // BAD [NOT DETECTED - TODO: extract Callables of @MainActor method calls]: HTML contains remote input, may access local secrets try! await sink(remoteString) // BAD [NOT DETECTED - TODO: extract Callables of @MainActor method calls] try! await sink("console.log(" + localStringFragment + ")") // GOOD: the HTML data is local @@ -201,7 +201,7 @@ func testSync(_ sink: @escaping (String) -> ()) { let remoteString = getRemoteData() sink(localString) // GOOD: the HTML data is local - sink(getRemoteData()) // BAD: HTML contains remote input, may access local secrets + sink(try! String(contentsOf: URL(string: "http://example.com/")!)) // BAD: HTML contains remote input, may access local secrets sink(remoteString) // BAD sink("console.log(" + localStringFragment + ")") // GOOD: the HTML data is local