mirror of
https://github.com/github/codeql.git
synced 2026-04-18 13:34:02 +02:00
Swift: Implement query.
This commit is contained in:
Geoffrey White
@@ -1,7 +1,7 @@
|
||||
/**
|
||||
* @name Unsafe WebView fetch
|
||||
* @description TODO
|
||||
* @kind problem
|
||||
* @kind path-problem
|
||||
* @problem.severity warning
|
||||
* @security-severity TODO
|
||||
* @precision high
|
||||
@@ -13,5 +13,116 @@
|
||||
*/
|
||||
|
||||
import swift
|
||||
import codeql.swift.dataflow.DataFlow
|
||||
import codeql.swift.dataflow.TaintTracking
|
||||
import codeql.swift.dataflow.FlowSources
|
||||
import DataFlow::PathGraph
|
||||
import codeql.swift.frameworks.StandardLibrary.String
|
||||
|
||||
select "TODO"
|
||||
/**
|
||||
* A taint source that is `String(contentsOf:)`.
|
||||
* TODO: this shouldn't be needed when `StringSource` in `String.qll` is working.
|
||||
*/
|
||||
class StringContentsOfURLSource extends RemoteFlowSource {
|
||||
StringContentsOfURLSource() {
|
||||
exists(CallExpr call, AbstractFunctionDecl f |
|
||||
call.getFunction().(ApplyExpr).getStaticTarget() = f and
|
||||
f.getName() = "init(contentsOf:)" and
|
||||
f.getParam(0).getType().getName() = "URL" and
|
||||
this.asExpr() = call
|
||||
)
|
||||
}
|
||||
|
||||
override string getSourceType() { result = "" }
|
||||
}
|
||||
|
||||
/**
|
||||
* A sink that is a candidate result for this query, such as certain arguments
|
||||
* to `UIWebView.loadHTMLString`.
|
||||
*/
|
||||
class Sink extends DataFlow::Node {
|
||||
Expr baseURL;
|
||||
|
||||
Sink() {
|
||||
exists(
|
||||
AbstractFunctionDecl funcDecl, CallExpr call, string funcName, string paramName, int arg,
|
||||
int baseURLarg
|
||||
|
|
||||
// arguments to method calls...
|
||||
exists(string className, ClassDecl c |
|
||||
(
|
||||
// `loadHTMLString`
|
||||
className = ["UIWebView", "WKWebView"] and
|
||||
funcName = "loadHTMLString(_:baseURL:)" and
|
||||
paramName = "string"
|
||||
or
|
||||
// `UIWebView.load`
|
||||
className = "UIWebView" and
|
||||
funcName = "load(_:mimeType:textEncodingName:baseURL:)" and
|
||||
paramName = "data"
|
||||
or
|
||||
// `WKWebView.load`
|
||||
className = "WKWebView" and
|
||||
funcName = "load(_:mimeType:characterEncodingName:baseURL:)" and
|
||||
paramName = "data"
|
||||
) and
|
||||
c.getName() = className and
|
||||
c.getAMember() = funcDecl and
|
||||
call.getFunction().(ApplyExpr).getStaticTarget() = funcDecl
|
||||
) and
|
||||
// match up `funcName`, `paramName`, `arg`, `node`.
|
||||
funcDecl.getName() = funcName and
|
||||
funcDecl.getParam(pragma[only_bind_into](arg)).getName() = paramName and
|
||||
call.getArgument(pragma[only_bind_into](arg)).getExpr() = this.asExpr() and
|
||||
// match up `baseURLArg`
|
||||
funcDecl.getParam(pragma[only_bind_into](baseURLarg)).getName() = "baseURL" and
|
||||
call.getArgument(pragma[only_bind_into](baseURLarg)).getExpr() = baseURL
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Gets the `baseURL` argument associated with this sink.
|
||||
*/
|
||||
Expr getBaseURL() { result = baseURL }
|
||||
}
|
||||
|
||||
/**
|
||||
* Taint configuration from taint sources to sinks (and `baseURL` arguments)
|
||||
* for this query.
|
||||
*/
|
||||
class UnsafeWebViewFetchConfig extends TaintTracking::Configuration {
|
||||
UnsafeWebViewFetchConfig() { this = "UnsafeWebViewFetchConfig" }
|
||||
|
||||
override predicate isSource(DataFlow::Node node) { node instanceof RemoteFlowSource }
|
||||
|
||||
override predicate isSink(DataFlow::Node node) {
|
||||
node instanceof Sink
|
||||
}
|
||||
|
||||
override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
|
||||
// allow flow through `try!` and similar constructs
|
||||
// TODO: this should probably be part of DataFlow / TaintTracking.
|
||||
node1.asExpr() = node2.asExpr().(AnyTryExpr).getSubExpr()
|
||||
or
|
||||
// allow flow through `!`
|
||||
// TODO: this should probably be part of DataFlow / TaintTracking.
|
||||
node1.asExpr() = node2.asExpr().(ForceValueExpr).getSubExpr()
|
||||
or
|
||||
// allow flow through string concatenation.
|
||||
// TODO: this should probably be part of TaintTracking.
|
||||
node2.asExpr().(AddExpr).getAnOperand() = node1.asExpr()
|
||||
}
|
||||
}
|
||||
|
||||
from
|
||||
UnsafeWebViewFetchConfig config, DataFlow::PathNode sourceNode, DataFlow::PathNode sinkNode,
|
||||
Sink sink, string message
|
||||
where
|
||||
config.hasFlowPath(sourceNode, sinkNode) and
|
||||
sink = sinkNode.getNode() and
|
||||
(
|
||||
// base URL is nil
|
||||
sink.getBaseURL() instanceof NilLiteralExpr and
|
||||
message = "Tainted data is used in a WebView fetch without restricting the base URL."
|
||||
)
|
||||
select sinkNode, sourceNode, sinkNode, message
|
||||
|
||||
@@ -1 +1,45 @@
|
||||
| TODO |
|
||||
edges
|
||||
| UnsafeWebViewFetch.swift:94:10:94:37 | try ... : | UnsafeWebViewFetch.swift:117:21:117:35 | call to getRemoteData() : |
|
||||
| UnsafeWebViewFetch.swift:94:10:94:37 | try ... : | UnsafeWebViewFetch.swift:120:25:120:39 | call to getRemoteData() |
|
||||
| UnsafeWebViewFetch.swift:94:10:94:37 | try ... : | UnsafeWebViewFetch.swift:164:21:164:35 | call to getRemoteData() : |
|
||||
| UnsafeWebViewFetch.swift:94:10:94:37 | try ... : | UnsafeWebViewFetch.swift:167:25:167:39 | call to getRemoteData() |
|
||||
| UnsafeWebViewFetch.swift:94:14:94:37 | call to ... : | UnsafeWebViewFetch.swift:94:10:94:37 | try ... : |
|
||||
| UnsafeWebViewFetch.swift:117:21:117:35 | call to getRemoteData() : | UnsafeWebViewFetch.swift:121:25:121:25 | remoteString |
|
||||
| UnsafeWebViewFetch.swift:117:21:117:35 | call to getRemoteData() : | UnsafeWebViewFetch.swift:124:25:124:51 | ... call to +(_:_:) ... |
|
||||
| UnsafeWebViewFetch.swift:117:21:117:35 | call to getRemoteData() : | UnsafeWebViewFetch.swift:135:25:135:25 | remoteString |
|
||||
| UnsafeWebViewFetch.swift:117:21:117:35 | call to getRemoteData() : | UnsafeWebViewFetch.swift:137:25:137:25 | remoteString |
|
||||
| UnsafeWebViewFetch.swift:117:21:117:35 | call to getRemoteData() : | UnsafeWebViewFetch.swift:139:25:139:25 | remoteString |
|
||||
| UnsafeWebViewFetch.swift:117:21:117:35 | call to getRemoteData() : | UnsafeWebViewFetch.swift:141:25:141:25 | remoteString |
|
||||
| UnsafeWebViewFetch.swift:164:21:164:35 | call to getRemoteData() : | UnsafeWebViewFetch.swift:168:25:168:25 | remoteString |
|
||||
| UnsafeWebViewFetch.swift:164:21:164:35 | call to getRemoteData() : | UnsafeWebViewFetch.swift:171:25:171:51 | ... call to +(_:_:) ... |
|
||||
| UnsafeWebViewFetch.swift:164:21:164:35 | call to getRemoteData() : | UnsafeWebViewFetch.swift:182:25:182:25 | remoteString |
|
||||
| UnsafeWebViewFetch.swift:164:21:164:35 | call to getRemoteData() : | UnsafeWebViewFetch.swift:184:25:184:25 | remoteString |
|
||||
| UnsafeWebViewFetch.swift:164:21:164:35 | call to getRemoteData() : | UnsafeWebViewFetch.swift:186:25:186:25 | remoteString |
|
||||
| UnsafeWebViewFetch.swift:164:21:164:35 | call to getRemoteData() : | UnsafeWebViewFetch.swift:188:25:188:25 | remoteString |
|
||||
nodes
|
||||
| UnsafeWebViewFetch.swift:94:10:94:37 | try ... : | semmle.label | try ... : |
|
||||
| UnsafeWebViewFetch.swift:94:14:94:37 | call to ... : | semmle.label | call to ... : |
|
||||
| UnsafeWebViewFetch.swift:117:21:117:35 | call to getRemoteData() : | semmle.label | call to getRemoteData() : |
|
||||
| UnsafeWebViewFetch.swift:120:25:120:39 | call to getRemoteData() | semmle.label | call to getRemoteData() |
|
||||
| UnsafeWebViewFetch.swift:121:25:121:25 | remoteString | semmle.label | remoteString |
|
||||
| UnsafeWebViewFetch.swift:124:25:124:51 | ... call to +(_:_:) ... | semmle.label | ... call to +(_:_:) ... |
|
||||
| UnsafeWebViewFetch.swift:135:25:135:25 | remoteString | semmle.label | remoteString |
|
||||
| UnsafeWebViewFetch.swift:137:25:137:25 | remoteString | semmle.label | remoteString |
|
||||
| UnsafeWebViewFetch.swift:139:25:139:25 | remoteString | semmle.label | remoteString |
|
||||
| UnsafeWebViewFetch.swift:141:25:141:25 | remoteString | semmle.label | remoteString |
|
||||
| UnsafeWebViewFetch.swift:164:21:164:35 | call to getRemoteData() : | semmle.label | call to getRemoteData() : |
|
||||
| UnsafeWebViewFetch.swift:167:25:167:39 | call to getRemoteData() | semmle.label | call to getRemoteData() |
|
||||
| UnsafeWebViewFetch.swift:168:25:168:25 | remoteString | semmle.label | remoteString |
|
||||
| UnsafeWebViewFetch.swift:171:25:171:51 | ... call to +(_:_:) ... | semmle.label | ... call to +(_:_:) ... |
|
||||
| UnsafeWebViewFetch.swift:182:25:182:25 | remoteString | semmle.label | remoteString |
|
||||
| UnsafeWebViewFetch.swift:184:25:184:25 | remoteString | semmle.label | remoteString |
|
||||
| UnsafeWebViewFetch.swift:186:25:186:25 | remoteString | semmle.label | remoteString |
|
||||
| UnsafeWebViewFetch.swift:188:25:188:25 | remoteString | semmle.label | remoteString |
|
||||
subpaths
|
||||
#select
|
||||
| UnsafeWebViewFetch.swift:120:25:120:39 | call to getRemoteData() | UnsafeWebViewFetch.swift:94:14:94:37 | call to ... : | UnsafeWebViewFetch.swift:120:25:120:39 | call to getRemoteData() | Tainted data is used in a WebView fetch without restricting the base URL. |
|
||||
| UnsafeWebViewFetch.swift:121:25:121:25 | remoteString | UnsafeWebViewFetch.swift:94:14:94:37 | call to ... : | UnsafeWebViewFetch.swift:121:25:121:25 | remoteString | Tainted data is used in a WebView fetch without restricting the base URL. |
|
||||
| UnsafeWebViewFetch.swift:124:25:124:51 | ... call to +(_:_:) ... | UnsafeWebViewFetch.swift:94:14:94:37 | call to ... : | UnsafeWebViewFetch.swift:124:25:124:51 | ... call to +(_:_:) ... | Tainted data is used in a WebView fetch without restricting the base URL. |
|
||||
| UnsafeWebViewFetch.swift:167:25:167:39 | call to getRemoteData() | UnsafeWebViewFetch.swift:94:14:94:37 | call to ... : | UnsafeWebViewFetch.swift:167:25:167:39 | call to getRemoteData() | Tainted data is used in a WebView fetch without restricting the base URL. |
|
||||
| UnsafeWebViewFetch.swift:168:25:168:25 | remoteString | UnsafeWebViewFetch.swift:94:14:94:37 | call to ... : | UnsafeWebViewFetch.swift:168:25:168:25 | remoteString | Tainted data is used in a WebView fetch without restricting the base URL. |
|
||||
| UnsafeWebViewFetch.swift:171:25:171:51 | ... call to +(_:_:) ... | UnsafeWebViewFetch.swift:94:14:94:37 | call to ... : | UnsafeWebViewFetch.swift:171:25:171:51 | ... call to +(_:_:) ... | Tainted data is used in a WebView fetch without restricting the base URL. |
|
||||
|
||||
@@ -117,11 +117,11 @@ func testUIWebView() {
|
||||
let remoteString = getRemoteData()
|
||||
|
||||
webview.loadHTMLString(localString, baseURL: nil) // GOOD: the HTML data is local
|
||||
webview.loadHTMLString(getRemoteData(), baseURL: nil) // BAD: HTML contains remote input, may access local secrets [NOT DETECTED]
|
||||
webview.loadHTMLString(remoteString, baseURL: nil) // BAD [NOT DETECTED]
|
||||
webview.loadHTMLString(getRemoteData(), baseURL: nil) // BAD: HTML contains remote input, may access local secrets
|
||||
webview.loadHTMLString(remoteString, baseURL: nil) // BAD
|
||||
|
||||
webview.loadHTMLString("<html>" + localStringFragment + "</html>", baseURL: nil) // GOOD: the HTML data is local
|
||||
webview.loadHTMLString("<html>" + remoteString + "</html>", baseURL: nil) // BAD [NOT DETECTED]
|
||||
webview.loadHTMLString("<html>" + remoteString + "</html>", baseURL: nil) // BAD
|
||||
|
||||
webview.loadHTMLString("<html>\(localStringFragment)</html>", baseURL: nil) // GOOD: the HTML data is local
|
||||
webview.loadHTMLString("<html>\(remoteString)</html>", baseURL: nil) // BAD [NOT DETECTED]
|
||||
@@ -164,11 +164,11 @@ func testWKWebView() {
|
||||
let remoteString = getRemoteData()
|
||||
|
||||
webview.loadHTMLString(localString, baseURL: nil) // GOOD: the HTML data is local
|
||||
webview.loadHTMLString(getRemoteData(), baseURL: nil) // BAD [NOT DETECTED]
|
||||
webview.loadHTMLString(remoteString, baseURL: nil) // BAD [NOT DETECTED]
|
||||
webview.loadHTMLString(getRemoteData(), baseURL: nil) // BAD
|
||||
webview.loadHTMLString(remoteString, baseURL: nil) // BAD
|
||||
|
||||
webview.loadHTMLString("<html>" + localStringFragment + "</html>", baseURL: nil) // GOOD: the HTML data is local
|
||||
webview.loadHTMLString("<html>" + remoteString + "</html>", baseURL: nil) // BAD [NOT DETECTED]
|
||||
webview.loadHTMLString("<html>" + remoteString + "</html>", baseURL: nil) // BAD
|
||||
|
||||
webview.loadHTMLString("<html>\(localStringFragment)</html>", baseURL: nil) // GOOD: the HTML data is local
|
||||
webview.loadHTMLString("<html>\(remoteString)</html>", baseURL: nil) // BAD [NOT DETECTED]
|
||||
|
||||
Reference in New Issue
Block a user