Skip to content

Commit baf7986

Browse files
committed
Rework types exported through JSContext
Better model the JSExport protocol logic
1 parent 48b0cc0 commit baf7986

File tree

3 files changed

+44
-29
lines changed

3 files changed

+44
-29
lines changed

swift/ql/lib/codeql/swift/frameworks/StandardLibrary/WebView.qll

Lines changed: 28 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -91,39 +91,43 @@ private class JsValueSummaries extends SummaryModelCsv {
9191
}
9292
}
9393

94-
/** The class `JSContext`. */
95-
private class JsContextDecl extends ClassDecl {
96-
JsContextDecl() { this.getName() = "JSContext" }
94+
/** The `JSExport` protocol. */
95+
private class JsExport extends ProtocolDecl {
96+
JsExport() { this.getName() = "JSExport" }
9797
}
9898

99-
/** The type of an object exposed to JavaScript through `JSContext.setObject`. */
100-
private class TypeExposedThroughJsContext extends Type {
101-
TypeExposedThroughJsContext() {
102-
exists(ApplyExpr c, FuncDecl f |
103-
c.getStaticTarget() = f and
104-
f.getEnclosingDecl() instanceof JsContextDecl and
105-
f.getName() = "setObject(_:forKeyedSubscript)"
106-
|
107-
c.getArgument(0).getExpr().getType() = this
108-
)
109-
}
99+
/** A protocol inheriting `JSExport`. */
100+
private class JsExportedProto extends ProtocolDecl {
101+
JsExportedProto() { this.getABaseTypeDecl+() instanceof JsExport }
102+
}
103+
104+
/** A type that adopts a `JSExport`-inherited protocol. */
105+
private class JsExportedType extends ClassOrStructDecl {
106+
JsExportedType() { this.getABaseTypeDecl*() instanceof JsExportedProto }
110107
}
111108

112109
/**
113-
* The members (fields and parameters of functions) of a class or struct
114-
* an instance of which is exposed to JavaScript through `JSContext.setObject`.
110+
* A flow source that models properties and methods defined in a `JSExport`-inherited protocol
111+
* and implemented in a type adopting that protcol. These members are accessible from JavaScript
112+
* when the object is assigned to a `JSContext`.
115113
*/
116-
private class ExposedThroughJsContextSource extends RemoteFlowSource {
117-
ExposedThroughJsContextSource() {
118-
exists(ParamDecl p | this.(DataFlow::ParameterNode).getParameter() = p |
119-
p.getDeclaringFunction().getEnclosingDecl().(ClassOrStructDecl).getType() instanceof
120-
TypeExposedThroughJsContext
114+
private class JsExportedSource extends RemoteFlowSource {
115+
JsExportedSource() {
116+
exists(MethodDecl adopter, MethodDecl base |
117+
base.getEnclosingDecl() instanceof JsExportedProto and
118+
adopter.getEnclosingDecl() instanceof JsExportedType
119+
|
120+
this.(DataFlow::ParameterNode).getParameter().getDeclaringFunction() = adopter and
121+
adopter.getName() = base.getName()
121122
)
122123
or
123-
exists(FieldDecl f | this.asDefinition().getSourceVariable() = f |
124-
f.getEnclosingDecl().(ClassOrStructDecl).getType() instanceof TypeExposedThroughJsContext
124+
exists(FieldDecl adopter, FieldDecl base |
125+
base.getEnclosingDecl() instanceof JsExportedProto and
126+
adopter.getEnclosingDecl() instanceof JsExportedType
127+
|
128+
this.asDefinition().getSourceVariable() = adopter and adopter.getName() = base.getName()
125129
)
126130
}
127131

128-
override string getSourceType() { result = "Member of a type exposed through JSContext" }
132+
override string getSourceType() { result = "Member of a type exposed through JSExport" }
129133
}

swift/ql/test/library-tests/dataflow/flowsources/FlowSources.expected

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,9 @@
88
| url.swift:53:15:53:19 | .resourceBytes | external |
99
| url.swift:60:15:60:19 | .lines | external |
1010
| url.swift:67:16:67:22 | .lines | external |
11-
| webview.swift:19:82:19:102 | message | external |
12-
| webview.swift:24:5:24:13 | .globalObject | external |
13-
| webview.swift:25:5:25:39 | call to objectForKeyedSubscript(_:) | external |
11+
| webview.swift:20:82:20:102 | message | external |
12+
| webview.swift:25:5:25:13 | .globalObject | external |
13+
| webview.swift:26:5:26:39 | call to objectForKeyedSubscript(_:) | external |
14+
| webview.swift:38:10:38:10 | self | Member of a type exposed through JSExport |
15+
| webview.swift:38:18:38:24 | arg1 | Member of a type exposed through JSExport |
16+
| webview.swift:38:29:38:35 | arg2 | Member of a type exposed through JSExport |

swift/ql/test/library-tests/dataflow/flowsources/webview.swift

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ class JSContext {
1313
func objectForKeyedSubscript(_: Any!) -> JSValue! { return JSValue() }
1414
func setObject(_: Any, forKeyedSubscript: (NSCopying & NSObjectProtocol) ) {}
1515
}
16+
protocol JSExport {}
1617

1718
// --- tests ---
1819
class TestMessageHandler: WKScriptMessageHandler {
@@ -23,13 +24,20 @@ class TestMessageHandler: WKScriptMessageHandler {
2324
func testJsContext(context: JSContext) {
2425
context.globalObject // SOURCE
2526
context.objectForKeyedSubscript("") // SOURCE
26-
context.setObject(Exposed.self, forKeyedSubscript: "exposed" as! NSCopying & NSObjectProtocol)
2727
}
2828

29-
class Exposed {
29+
protocol Exported : JSExport {
30+
var tainted: Any { get }
31+
func tainted(arg1: Any, arg2: Any)
32+
}
33+
class ExportedImpl : Exported {
3034
var tainted: Any { get { return "" } } // SOURCE
3135

36+
var notTainted: Any { get { return ""} }
37+
3238
func tainted(arg1: Any, arg2: Any) { // SOURCES
39+
}
3340

41+
func notTainted(arg1: Any, arg2: Any) {
3442
}
3543
}

0 commit comments

Comments
 (0)