|
1 | 1 | /** |
2 | 2 | * @name Unsafe WebView fetch |
3 | 3 | * @description TODO |
4 | | - * @kind problem |
| 4 | + * @kind path-problem |
5 | 5 | * @problem.severity warning |
6 | 6 | * @security-severity TODO |
7 | 7 | * @precision high |
|
13 | 13 | */ |
14 | 14 |
|
15 | 15 | import swift |
| 16 | +import codeql.swift.dataflow.DataFlow |
| 17 | +import codeql.swift.dataflow.TaintTracking |
| 18 | +import codeql.swift.dataflow.FlowSources |
| 19 | +import DataFlow::PathGraph |
| 20 | +import codeql.swift.frameworks.StandardLibrary.String |
16 | 21 |
|
17 | | -select "TODO" |
| 22 | +/** |
| 23 | + * A taint source that is `String(contentsOf:)`. |
| 24 | + * TODO: this shouldn't be needed when `StringSource` in `String.qll` is working. |
| 25 | + */ |
| 26 | +class StringContentsOfURLSource extends RemoteFlowSource { |
| 27 | + StringContentsOfURLSource() { |
| 28 | + exists(CallExpr call, AbstractFunctionDecl f | |
| 29 | + call.getFunction().(ApplyExpr).getStaticTarget() = f and |
| 30 | + f.getName() = "init(contentsOf:)" and |
| 31 | + f.getParam(0).getType().getName() = "URL" and |
| 32 | + this.asExpr() = call |
| 33 | + ) |
| 34 | + } |
| 35 | + |
| 36 | + override string getSourceType() { result = "" } |
| 37 | +} |
| 38 | + |
| 39 | +/** |
| 40 | + * A sink that is a candidate result for this query, such as certain arguments |
| 41 | + * to `UIWebView.loadHTMLString`. |
| 42 | + */ |
| 43 | +class Sink extends DataFlow::Node { |
| 44 | + Expr baseURL; |
| 45 | + |
| 46 | + Sink() { |
| 47 | + exists( |
| 48 | + AbstractFunctionDecl funcDecl, CallExpr call, string funcName, string paramName, int arg, |
| 49 | + int baseURLarg |
| 50 | + | |
| 51 | + // arguments to method calls... |
| 52 | + exists(string className, ClassDecl c | |
| 53 | + ( |
| 54 | + // `loadHTMLString` |
| 55 | + className = ["UIWebView", "WKWebView"] and |
| 56 | + funcName = "loadHTMLString(_:baseURL:)" and |
| 57 | + paramName = "string" |
| 58 | + or |
| 59 | + // `UIWebView.load` |
| 60 | + className = "UIWebView" and |
| 61 | + funcName = "load(_:mimeType:textEncodingName:baseURL:)" and |
| 62 | + paramName = "data" |
| 63 | + or |
| 64 | + // `WKWebView.load` |
| 65 | + className = "WKWebView" and |
| 66 | + funcName = "load(_:mimeType:characterEncodingName:baseURL:)" and |
| 67 | + paramName = "data" |
| 68 | + ) and |
| 69 | + c.getName() = className and |
| 70 | + c.getAMember() = funcDecl and |
| 71 | + call.getFunction().(ApplyExpr).getStaticTarget() = funcDecl |
| 72 | + ) and |
| 73 | + // match up `funcName`, `paramName`, `arg`, `node`. |
| 74 | + funcDecl.getName() = funcName and |
| 75 | + funcDecl.getParam(pragma[only_bind_into](arg)).getName() = paramName and |
| 76 | + call.getArgument(pragma[only_bind_into](arg)).getExpr() = this.asExpr() and |
| 77 | + // match up `baseURLArg` |
| 78 | + funcDecl.getParam(pragma[only_bind_into](baseURLarg)).getName() = "baseURL" and |
| 79 | + call.getArgument(pragma[only_bind_into](baseURLarg)).getExpr() = baseURL |
| 80 | + ) |
| 81 | + } |
| 82 | + |
| 83 | + /** |
| 84 | + * Gets the `baseURL` argument associated with this sink. |
| 85 | + */ |
| 86 | + Expr getBaseURL() { result = baseURL } |
| 87 | +} |
| 88 | + |
| 89 | +/** |
| 90 | + * Taint configuration from taint sources to sinks (and `baseURL` arguments) |
| 91 | + * for this query. |
| 92 | + */ |
| 93 | +class UnsafeWebViewFetchConfig extends TaintTracking::Configuration { |
| 94 | + UnsafeWebViewFetchConfig() { this = "UnsafeWebViewFetchConfig" } |
| 95 | + |
| 96 | + override predicate isSource(DataFlow::Node node) { node instanceof RemoteFlowSource } |
| 97 | + |
| 98 | + override predicate isSink(DataFlow::Node node) { |
| 99 | + node instanceof Sink |
| 100 | + } |
| 101 | + |
| 102 | + override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) { |
| 103 | + // allow flow through `try!` and similar constructs |
| 104 | + // TODO: this should probably be part of DataFlow / TaintTracking. |
| 105 | + node1.asExpr() = node2.asExpr().(AnyTryExpr).getSubExpr() |
| 106 | + or |
| 107 | + // allow flow through `!` |
| 108 | + // TODO: this should probably be part of DataFlow / TaintTracking. |
| 109 | + node1.asExpr() = node2.asExpr().(ForceValueExpr).getSubExpr() |
| 110 | + or |
| 111 | + // allow flow through string concatenation. |
| 112 | + // TODO: this should probably be part of TaintTracking. |
| 113 | + node2.asExpr().(AddExpr).getAnOperand() = node1.asExpr() |
| 114 | + } |
| 115 | +} |
| 116 | + |
| 117 | +from |
| 118 | + UnsafeWebViewFetchConfig config, DataFlow::PathNode sourceNode, DataFlow::PathNode sinkNode, |
| 119 | + Sink sink, string message |
| 120 | +where |
| 121 | + config.hasFlowPath(sourceNode, sinkNode) and |
| 122 | + sink = sinkNode.getNode() and |
| 123 | + ( |
| 124 | + // base URL is nil |
| 125 | + sink.getBaseURL() instanceof NilLiteralExpr and |
| 126 | + message = "Tainted data is used in a WebView fetch without restricting the base URL." |
| 127 | + ) |
| 128 | +select sinkNode, sourceNode, sinkNode, message |
0 commit comments