Skip to content

Commit 1e48500

Browse files
atorralbaatorralba
committed
Increase precision of the URL(string:relativeTo:) models
1 parent 4b2aa93 commit 1e48500

File tree

2 files changed

+53
-23
lines changed
  • swift/ql
    • lib/codeql/swift/frameworks/StandardLibrary
    • test/library-tests/dataflow/taint

2 files changed

+53
-23
lines changed

‎swift/ql/lib/codeql/swift/frameworks/StandardLibrary/Url.qll

Lines changed: 31 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,9 @@ private class UrlSummaries extends SummaryModelCsv {
2222
row =
2323
[
2424
";URL;true;init(string:);(String);;Argument[0];ReturnValue;taint",
25+
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[0,1];ReturnValue;taint",
26+
// The base string taints all the URL fields (except baseURL)
2527
";URL;true;init(string:);(String);;Argument[0];ReturnValue.Field[absoluteURL];taint",
26-
";URL;true;init(string:);(String);;Argument[0];ReturnValue.Field[baseURL];taint",
2728
";URL;true;init(string:);(String);;Argument[0];ReturnValue.Field[fragment];taint",
2829
";URL;true;init(string:);(String);;Argument[0];ReturnValue.Field[host];taint",
2930
";URL;true;init(string:);(String);;Argument[0];ReturnValue.Field[lastPathComponent];taint",
@@ -39,24 +40,35 @@ private class UrlSummaries extends SummaryModelCsv {
3940
";URL;true;init(string:);(String);;Argument[0];ReturnValue.Field[standardizedFileURL];taint",
4041
";URL;true;init(string:);(String);;Argument[0];ReturnValue.Field[user];taint",
4142
";URL;true;init(string:);(String);;Argument[0];ReturnValue.Field[password];taint",
42-
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[0,1];ReturnValue;taint",
43-
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[0,1];ReturnValue.Field[absoluteURL];taint",
44-
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[0,1];ReturnValue.Field[baseURL];taint",
45-
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[0,1];ReturnValue.Field[fragment];taint",
46-
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[0,1];ReturnValue.Field[host];taint",
47-
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[0,1];ReturnValue.Field[lastPathComponent];taint",
48-
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[0,1];ReturnValue.Field[path];taint",
49-
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[0,1];ReturnValue.Field[pathComponents];taint",
50-
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[0,1];ReturnValue.Field[pathExtension];taint",
51-
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[0,1];ReturnValue.Field[port];taint",
52-
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[0,1];ReturnValue.Field[query];taint",
53-
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[0,1];ReturnValue.Field[relativePath];taint",
54-
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[0,1];ReturnValue.Field[relativeString];taint",
55-
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[0,1];ReturnValue.Field[scheme];taint",
56-
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[0,1];ReturnValue.Field[standardized];taint",
57-
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[0,1];ReturnValue.Field[standardizedFileURL];taint",
58-
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[0,1];ReturnValue.Field[user];taint",
59-
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[0,1];ReturnValue.Field[password];taint",
43+
// The base string taints all the URL fields (except baseURL) if it's an absolute URL when relativeTo is used
44+
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[0];ReturnValue.Field[absoluteURL];taint",
45+
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[0];ReturnValue.Field[fragment];taint",
46+
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[0];ReturnValue.Field[host];taint",
47+
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[0];ReturnValue.Field[lastPathComponent];taint",
48+
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[0];ReturnValue.Field[path];taint",
49+
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[0];ReturnValue.Field[pathComponents];taint",
50+
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[0];ReturnValue.Field[pathExtension];taint",
51+
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[0];ReturnValue.Field[port];taint",
52+
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[0];ReturnValue.Field[query];taint",
53+
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[0];ReturnValue.Field[relativePath];taint",
54+
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[0];ReturnValue.Field[relativeString];taint",
55+
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[0];ReturnValue.Field[scheme];taint",
56+
// Not mapping precise field taint to standardized/standardizedFileURL even if the return values are URLs too
57+
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[0];ReturnValue.Field[standardized];taint",
58+
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[0];ReturnValue.Field[standardizedFileURL];taint",
59+
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[0];ReturnValue.Field[user];taint",
60+
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[0];ReturnValue.Field[password];taint",
61+
// The relativeTo URL taints fields not related to the path, query or fragment if the base string is a relative path
62+
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[1];ReturnValue.Field[absoluteURL];taint",
63+
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[1];ReturnValue.Field[baseURL];taint",
64+
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[1];ReturnValue.Field[host];taint",
65+
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[1];ReturnValue.Field[port];taint",
66+
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[1];ReturnValue.Field[scheme];taint",
67+
// Not mapping precise field taint to standardized/standardizedFileURL even if the return values are URLs too
68+
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[1];ReturnValue.Field[standardized];taint",
69+
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[1];ReturnValue.Field[standardizedFileURL];taint",
70+
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[1];ReturnValue.Field[user];taint",
71+
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[1];ReturnValue.Field[password];taint",
6072
]
6173
}
6274
}

‎swift/ql/test/library-tests/dataflow/taint/url.swift

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ struct URL
2020
var relativePath: String { get {return ""} }
2121
var relativeString: String { get {return ""} }
2222
var scheme: String? { get {return nil} }
23-
var standardized: String { get {return ""} }
23+
var standardized: URL { get {return URL(string: "")!} }
2424
var standardizedFileURL: URL { get {return URL(string: "")!} }
2525
var user: String? { get {return nil} }
2626
var password: String? { get {return nil} }
@@ -61,7 +61,8 @@ func taintThroughURL() {
6161
sink(arg: urlClean)
6262
sink(arg: urlTainted) // $ tainted=57
6363
sink(arg: urlTainted.absoluteURL) // $ tainted=57
64-
sink(arg: urlTainted.baseURL) // $ tainted=57
64+
sink(arg: urlTainted.baseURL) // $ Safe
65+
// Fields
6566
sink(string: urlTainted.fragment!) // $ tainted=57
6667
sink(string: urlTainted.host!) // $ tainted=57
6768
sink(string: urlTainted.lastPathComponent) // $ tainted=57
@@ -73,15 +74,32 @@ func taintThroughURL() {
7374
sink(string: urlTainted.relativePath) // $ tainted=57
7475
sink(string: urlTainted.relativeString) // $ tainted=57
7576
sink(string: urlTainted.scheme!) // $ tainted=57
76-
sink(string: urlTainted.standardized) // $ tainted=57
77+
sink(arg: urlTainted.standardized) // $ tainted=57
7778
sink(arg: urlTainted.standardizedFileURL) // $ tainted=57
7879
sink(string: urlTainted.user!) // $ tainted=57
7980
sink(string: urlTainted.password!) // $ tainted=57
8081

8182
sink(arg: URL(string: clean, relativeTo: nil)!)
8283
sink(arg: URL(string: tainted, relativeTo: nil)!) // $ tainted=57
8384
sink(arg: URL(string: clean, relativeTo: urlClean)!)
84-
sink(arg: URL(string: clean, relativeTo: urlTainted)!) // $ tainted=57
85+
// Fields (assuming `clean` was a relative path instead of a full URL)
86+
sink(arg: URL(string: clean, relativeTo: urlTainted)!.absoluteURL) // $ tainted=57
87+
sink(arg: URL(string: clean, relativeTo: urlTainted)!.baseURL) // $ tainted=57
88+
sink(string: URL(string: clean, relativeTo: urlTainted)!.fragment!) // Safe
89+
sink(string: URL(string: clean, relativeTo: urlTainted)!.host!) // $ tainted=57
90+
sink(string: URL(string: clean, relativeTo: urlTainted)!.lastPathComponent) // Safe
91+
sink(string: URL(string: clean, relativeTo: urlTainted)!.path) // Safe
92+
sink(string: URL(string: clean, relativeTo: urlTainted)!.pathComponents[0]) // Safe
93+
sink(string: URL(string: clean, relativeTo: urlTainted)!.pathExtension) // Safe
94+
sink(int: URL(string: clean, relativeTo: urlTainted)!.port!) // $ tainted=57
95+
sink(string: URL(string: clean, relativeTo: urlTainted)!.query!) // Safe
96+
sink(string: URL(string: clean, relativeTo: urlTainted)!.relativePath) // Safe
97+
sink(string: URL(string: clean, relativeTo: urlTainted)!.relativeString) // Safe
98+
sink(string: URL(string: clean, relativeTo: urlTainted)!.scheme!) // $ tainted=57
99+
sink(arg: URL(string: clean, relativeTo: urlTainted)!.standardized) // $ tainted=57
100+
sink(arg: URL(string: clean, relativeTo: urlTainted)!.standardizedFileURL) // $ tainted=57
101+
sink(string: URL(string: clean, relativeTo: urlTainted)!.user!) // $ tainted=57
102+
sink(string: URL(string: clean, relativeTo: urlTainted)!.password!) // $ tainted=57
85103

86104
if let x = URL(string: clean) {
87105
sink(arg: x)

0 commit comments

Comments
 (0)