Skip to content

Commit cd14703

Browse files
committed
Swift: Fill some gaps in the URL, NSURL models.
1 parent a86862d commit cd14703

File tree

3 files changed

+30
-6
lines changed

3 files changed

+30
-6
lines changed

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

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,34 @@
33
*/
44

55
import swift
6+
private import codeql.swift.dataflow.DataFlow
67
private import codeql.swift.dataflow.ExternalFlow
8+
private import codeql.swift.dataflow.FlowSteps
9+
10+
/**
11+
* A content implying that, if an `NSURL` is tainted, then all its fields are tainted.
12+
*/
13+
private class NSUrlFieldsInheritTaint extends TaintInheritingContent,
14+
DataFlow::Content::FieldContent
15+
{
16+
NSUrlFieldsInheritTaint() {
17+
this.getField().getEnclosingDecl().asNominalTypeDecl().getFullName() = "NSURL"
18+
}
19+
}
720

821
/**
922
* A model for `NSURL` members that permit taint flow.
1023
*/
1124
private class NsUrlSummaries extends SummaryModelCsv {
1225
override predicate row(string row) {
13-
row = ";NSURL;true;init(string:);(String);;Argument[0];ReturnValue.OptionalSome;taint"
26+
row =
27+
[
28+
";NSURL;true;init(string:);(String);;Argument[0];ReturnValue.OptionalSome;taint",
29+
";NSURL;true;appendingPathComponent(_:);;;Argument[-1..0];ReturnValue;taint",
30+
";NSURL;true;appendingPathComponent(_:isDirectory:);;;Argument[-1..0];ReturnValue;taint",
31+
";NSURL;true;appendingPathComponent(_:conformingTo:);;;Argument[-1..0];ReturnValue;taint",
32+
";NSURL;true;appendingPathExtension(_:);;;Argument[-1..0];ReturnValue;taint",
33+
";NSURL;true;appendingPathExtension(for:);;;Argument[-1];ReturnValue;taint",
34+
]
1435
}
1536
}

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,8 @@ class UrlDecl extends StructDecl {
1515
/**
1616
* A content implying that, if a `URL` is tainted, then all its fields are tainted.
1717
*/
18-
private class UriFieldsInheritTaint extends TaintInheritingContent, DataFlow::Content::FieldContent {
19-
UriFieldsInheritTaint() {
18+
private class UrlFieldsInheritTaint extends TaintInheritingContent, DataFlow::Content::FieldContent {
19+
UrlFieldsInheritTaint() {
2020
this.getField().getEnclosingDecl().asNominalTypeDecl() instanceof UrlDecl
2121
}
2222
}
@@ -106,6 +106,8 @@ private class UrlSummaries extends SummaryModelCsv {
106106
";URL;true;init(dataRepresentation:relativeTo:isAbsolute:);;;Argument[0];ReturnValue;taint",
107107
";URL;true;init(dataRepresentation:relativeTo:isAbsolute:);;;Argument[1].OptionalSome;ReturnValue;taint",
108108
";URL;true;init(_:strategy:);;;Argument[0];ReturnValue;taint",
109+
";URL;true;init(filePath:);;;Argument[0];ReturnValue.OptionalSome;taint",
110+
";URL;true;init(filePath:isDirectory:);;;Argument[0];ReturnValue.OptionalSome;taint",
109111
";URL;true;init(filePath:directoryHint:);;;Argument[0];ReturnValue.OptionalSome;taint",
110112
";URL;true;init(filePath:directoryHint:relativeTo:);;;Argument[0];ReturnValue;taint",
111113
";URL;true;init(filePath:directoryHint:relativeTo:);;;Argument[2].OptionalSome;ReturnValue;taint",
@@ -126,6 +128,7 @@ private class UrlSummaries extends SummaryModelCsv {
126128
";URL;true;appendingPathComponent(_:conformingTo:);;;Argument[-1..0];ReturnValue;taint",
127129
";URL;true;appendPathExtension(_:);;;Argument[-1..0];Argument[-1];taint",
128130
";URL;true;appendingPathExtension(_:);;;Argument[-1..0];ReturnValue;taint",
131+
";URL;true;appendingPathExtension(for:);;;Argument[-1];ReturnValue;taint",
129132
";URL;true;deletingLastPathComponent();;;Argument[-1];ReturnValue;taint",
130133
";URL;true;deletingPathExtension();;;Argument[-1];ReturnValue;taint",
131134
";URL;true;bookmarkData(options:includingResourceValuesForKeys:relativeTo:);;;Argument[-1];ReturnValue;taint",

swift/ql/test/query-tests/Security/CWE-022/testPathInjection.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -458,11 +458,11 @@ func testPathInjection2(s1: UnsafeMutablePointer<String>, s2: UnsafeMutablePoint
458458
let u3 = NSURL(string: "")!
459459
Data("").write(to: u3.filePathURL!, options: [])
460460
Data("").write(to: u3.appendingPathComponent("")!, options: [])
461-
Data("").write(to: u3.appendingPathComponent(remoteString)!, options: []) // $ MISSING: hasPathInjection=445
461+
Data("").write(to: u3.appendingPathComponent(remoteString)!, options: []) // $ hasPathInjection=445
462462

463463
let u4 = NSURL(string: remoteString)!
464-
Data("").write(to: u4.filePathURL!, options: []) // $ MISSING: hasPathInjection=445
465-
Data("").write(to: u4.appendingPathComponent("")!, options: []) // $ MISSING: hasPathInjection=445
464+
Data("").write(to: u4.filePathURL!, options: []) // $ hasPathInjection=445
465+
Data("").write(to: u4.appendingPathComponent("")!, options: []) // $ hasPathInjection=445
466466

467467
_ = NSData(contentsOfFile: remoteString)! // $ MISSING: hasPathInjection=445
468468
_ = NSData(contentsOfMappedFile: remoteString)! // $ MISSING: hasPathInjection=445

0 commit comments

Comments
 (0)