Skip to content

Commit fd226c5

Browse files
redsun82redsun82
authored
Merge pull request #10924 from github/redsun82/swift-fix-qltest-failure-reporting
Swift: add qltest tests and fix its failure reporting
2 parents e566357 + 408968a commit fd226c5

File tree

26 files changed

+289
-109
lines changed

26 files changed

+289
-109
lines changed

‎.github/workflows/swift-qltest.yml‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,23 @@ jobs:
2323
- uses: ./.github/actions/fetch-codeql
2424
- name: Check QL formatting
2525
run: find ql "(" -name "*.ql" -or -name "*.qll" ")" -print0 | xargs -0 codeql query format --check-only
26+
qltest-test:
27+
runs-on: ubuntu-latest
28+
steps:
29+
- uses: actions/checkout@v3
30+
- uses: bazelbuild/setup-bazelisk@v2
31+
- uses: actions/setup-python@v4
32+
with:
33+
python-version-file: 'swift/.python-version'
34+
- name: Test qltest.sh
35+
run: |
36+
bazel test //swift/tools/test/qltest
2637
qltest:
2738
runs-on: ${{ matrix.os }}
2839
strategy:
2940
fail-fast: false
3041
matrix:
31-
os : [ubuntu-20.04, macos-latest]
42+
os: [ ubuntu-20.04, macos-latest ]
3243
steps:
3344
- uses: actions/checkout@v3
3445
- uses: ./.github/actions/fetch-codeql

‎swift/BUILD.bazel‎

Lines changed: 4 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -23,19 +23,6 @@ pkg_files(
2323
],
2424
)
2525

26-
pkg_files(
27-
name = "scripts",
28-
srcs = ["tools/qltest.sh", "tools/autobuild.sh"],
29-
attributes = pkg_attributes(mode = "0755"),
30-
prefix = "tools",
31-
)
32-
33-
pkg_files(
34-
name = "tracing-config",
35-
srcs = ["tools/tracing-config.lua"],
36-
prefix = "tools",
37-
)
38-
3926
pkg_files(
4027
name = "manifest",
4128
srcs = ["codeql-extractor.yml"],
@@ -46,8 +33,7 @@ pkg_filegroup(
4633
srcs = [
4734
":dbscheme_files",
4835
":manifest",
49-
":tracing-config",
50-
":scripts",
36+
"//swift/tools",
5137
],
5238
visibility = ["//visibility:public"],
5339
)
@@ -79,7 +65,7 @@ pkg_filegroup(
7965
] + select({
8066
"@platforms//os:linux": [],
8167
"@platforms//os:macos": [
82-
":xcode-autobuilder"
68+
":xcode-autobuilder",
8369
],
8470
}),
8571
visibility = ["//visibility:public"],
@@ -101,7 +87,7 @@ pkg_install(
10187

10288
py_binary(
10389
name = "create-extractor-pack",
104-
srcs = ["tools/create_extractor_pack.py"],
105-
main = "tools/create_extractor_pack.py",
90+
srcs = ["create_extractor_pack.py"],
91+
main = "create_extractor_pack.py",
10692
deps = [":_create_extractor_pack"],
10793
)

‎swift/tools/create_extractor_pack.py‎ renamed to ‎swift/create_extractor_pack.py‎

File renamed without changes.

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1-
| customurlschemes.swift:17:44:17:54 | url | external |
2-
| customurlschemes.swift:20:52:20:68 | url | external |
3-
| customurlschemes.swift:23:52:23:62 | url | external |
1+
| customurlschemes.swift:23:44:23:54 | url | external |
2+
| customurlschemes.swift:27:52:27:68 | url | external |
3+
| customurlschemes.swift:31:52:31:62 | url | external |
44
| string.swift:27:21:27:21 | call to init(contentsOf:) | external |
55
| string.swift:27:21:27:44 | call to init(contentsOf:) | external |
66
| url.swift:53:15:53:19 | .resourceBytes | external |

‎swift/ql/test/library-tests/dataflow/flowsources/customurlschemes.swift‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,35 @@
11
// --- stubs ---
22
class UIApplication {
3-
struct OpenURLOptionsKey {}
3+
struct OpenURLOptionsKey : Hashable {
4+
static func == (lhs: OpenURLOptionsKey, rhs: OpenURLOptionsKey) -> Bool {
5+
return true;
6+
}
7+
8+
func hash(into hasher: inout Hasher) {}
9+
}
410
}
511

612
struct URL {}
713

814
protocol UIApplicationDelegate {
9-
optional func application(_ app: UIApplication, open url: URL, options: [UIApplication.OpenURLOptionsKey : Any]) -> Bool
10-
optional func application(_ application: UIApplication, handleOpen url: URL) -> Bool
11-
optional func application(_ application: UIApplication, open url: URL, sourceApplication: String?, annotation: Any) -> Bool
15+
func application(_ app: UIApplication, open url: URL, options: [UIApplication.OpenURLOptionsKey : Any]) -> Bool
16+
func application(_ application: UIApplication, handleOpen url: URL) -> Bool
17+
func application(_ application: UIApplication, open url: URL, sourceApplication: String?, annotation: Any) -> Bool
1218
}
1319

1420
// --- tests ---
1521

1622
class AppDelegate: UIApplicationDelegate {
1723
func application(_ app: UIApplication, open url: URL, options: [UIApplication.OpenURLOptionsKey : Any]) -> Bool { // SOURCE
24+
return true;
1825
}
1926

2027
func application(_ application: UIApplication, handleOpen url: URL) -> Bool { // SOURCE
28+
return true;
2129
}
2230

2331
func application(_ application: UIApplication, open url: URL, sourceApplication: String?, annotation: Any) -> Bool { // SOURCE
32+
return true;
2433
}
2534

26-
}
35+
}

‎swift/ql/test/library-tests/dataflow/taint/LocalTaint.expected‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -123,8 +123,8 @@
123123
| string.swift:39:13:39:19 | ... .+(_:_:) ... | string.swift:39:13:39:29 | ... .+(_:_:) ... |
124124
| string.swift:39:19:39:19 | tainted | string.swift:39:13:39:19 | ... .+(_:_:) ... |
125125
| string.swift:39:29:39:29 | < | string.swift:39:13:39:29 | ... .+(_:_:) ... |
126-
| subscript.swift:13:10:13:17 | call to source() | subscript.swift:13:10:13:20 | ...[...] |
127-
| subscript.swift:14:10:14:18 | call to source2() | subscript.swift:14:10:14:21 | ...[...] |
126+
| subscript.swift:13:15:13:22 | call to source() | subscript.swift:13:15:13:25 | ...[...] |
127+
| subscript.swift:14:15:14:23 | call to source2() | subscript.swift:14:15:14:26 | ...[...] |
128128
| url.swift:64:12:64:12 | urlTainted | url.swift:64:12:64:23 | .absoluteURL |
129129
| url.swift:65:12:65:12 | urlTainted | url.swift:65:12:65:23 | .baseURL |
130130
| url.swift:66:15:66:15 | urlTainted | url.swift:66:15:66:26 | .fragment |

‎swift/ql/test/library-tests/dataflow/taint/Taint.expected‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@ edges
1010
| string.swift:28:17:28:25 | call to source2() : | string.swift:35:13:35:23 | ... .+(_:_:) ... |
1111
| string.swift:28:17:28:25 | call to source2() : | string.swift:36:13:36:23 | ... .+(_:_:) ... |
1212
| string.swift:28:17:28:25 | call to source2() : | string.swift:39:13:39:29 | ... .+(_:_:) ... |
13-
| subscript.swift:13:10:13:17 | call to source() : | subscript.swift:13:10:13:20 | ...[...] |
14-
| subscript.swift:14:10:14:18 | call to source2() : | subscript.swift:14:10:14:21 | ...[...] |
13+
| subscript.swift:13:15:13:22 | call to source() : | subscript.swift:13:15:13:25 | ...[...] |
14+
| subscript.swift:14:15:14:23 | call to source2() : | subscript.swift:14:15:14:26 | ...[...] |
1515
| try.swift:9:17:9:24 | call to source() : | try.swift:9:13:9:24 | try ... |
1616
| try.swift:15:17:15:24 | call to source() : | try.swift:15:12:15:24 | try! ... |
1717
| try.swift:18:18:18:25 | call to source() : | try.swift:18:12:18:27 | ...! |
@@ -164,10 +164,10 @@ nodes
164164
| string.swift:35:13:35:23 | ... .+(_:_:) ... | semmle.label | ... .+(_:_:) ... |
165165
| string.swift:36:13:36:23 | ... .+(_:_:) ... | semmle.label | ... .+(_:_:) ... |
166166
| string.swift:39:13:39:29 | ... .+(_:_:) ... | semmle.label | ... .+(_:_:) ... |
167-
| subscript.swift:13:10:13:17 | call to source() : | semmle.label | call to source() : |
168-
| subscript.swift:13:10:13:20 | ...[...] | semmle.label | ...[...] |
169-
| subscript.swift:14:10:14:18 | call to source2() : | semmle.label | call to source2() : |
170-
| subscript.swift:14:10:14:21 | ...[...] | semmle.label | ...[...] |
167+
| subscript.swift:13:15:13:22 | call to source() : | semmle.label | call to source() : |
168+
| subscript.swift:13:15:13:25 | ...[...] | semmle.label | ...[...] |
169+
| subscript.swift:14:15:14:23 | call to source2() : | semmle.label | call to source2() : |
170+
| subscript.swift:14:15:14:26 | ...[...] | semmle.label | ...[...] |
171171
| try.swift:9:13:9:24 | try ... | semmle.label | try ... |
172172
| try.swift:9:17:9:24 | call to source() : | semmle.label | call to source() : |
173173
| try.swift:15:12:15:24 | try! ... | semmle.label | try! ... |
@@ -315,8 +315,8 @@ subpaths
315315
| string.swift:35:13:35:23 | ... .+(_:_:) ... | string.swift:28:17:28:25 | call to source2() : | string.swift:35:13:35:23 | ... .+(_:_:) ... | result |
316316
| string.swift:36:13:36:23 | ... .+(_:_:) ... | string.swift:28:17:28:25 | call to source2() : | string.swift:36:13:36:23 | ... .+(_:_:) ... | result |
317317
| string.swift:39:13:39:29 | ... .+(_:_:) ... | string.swift:28:17:28:25 | call to source2() : | string.swift:39:13:39:29 | ... .+(_:_:) ... | result |
318-
| subscript.swift:13:10:13:20 | ...[...] | subscript.swift:13:10:13:17 | call to source() : | subscript.swift:13:10:13:20 | ...[...] | result |
319-
| subscript.swift:14:10:14:21 | ...[...] | subscript.swift:14:10:14:18 | call to source2() : | subscript.swift:14:10:14:21 | ...[...] | result |
318+
| subscript.swift:13:15:13:25 | ...[...] | subscript.swift:13:15:13:22 | call to source() : | subscript.swift:13:15:13:25 | ...[...] | result |
319+
| subscript.swift:14:15:14:26 | ...[...] | subscript.swift:14:15:14:23 | call to source2() : | subscript.swift:14:15:14:26 | ...[...] | result |
320320
| try.swift:9:13:9:24 | try ... | try.swift:9:17:9:24 | call to source() : | try.swift:9:13:9:24 | try ... | result |
321321
| try.swift:15:12:15:24 | try! ... | try.swift:15:17:15:24 | call to source() : | try.swift:15:12:15:24 | try! ... | result |
322322
| try.swift:18:12:18:27 | ...! | try.swift:18:18:18:25 | call to source() : | try.swift:18:12:18:27 | ...! | result |

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,14 @@ class Data
44
init<S>(_ elements: S) {}
55
}
66

7+
extension String {
8+
struct Encoding {
9+
static let utf8 = Encoding()
10+
}
11+
12+
init?(data: Data, encoding: Encoding) { self.init() }
13+
}
14+
715
func source() -> String { return "" }
816
func sink(arg: Data) {}
917
func sink2(arg: String) {}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,6 @@ func source2() -> SubscriptTest { return SubscriptTest() }
1010
func sink(arg: String) {}
1111

1212
func test() {
13-
sink(source()[0]) // $ tainted=13
14-
sink(source2()[0]) // $ tainted=14
13+
sink(arg: source()[0]) // $ tainted=13
14+
sink(arg: source2()[0]) // $ tainted=14
1515
}

‎swift/ql/test/query-tests/Security/CWE-089/SQLite.swift‎

Lines changed: 28 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,13 @@ class Statement {
2626

2727
init(_ connection: Connection, _ SQL: String) throws { self.connection = connection}
2828

29-
public func bind(_ values: Binding?...) -> Statement { return Statement(connection, "") }
30-
public func bind(_ values: [Binding?]) -> Statement { return Statement(connection, "") }
31-
public func bind(_ values: [String: Binding?]) -> Statement { return Statement(connection, "") }
29+
public func bind(_ values: Binding?...) -> Statement { return self }
30+
public func bind(_ values: [Binding?]) -> Statement { return self }
31+
public func bind(_ values: [String: Binding?]) -> Statement { return self }
3232

33-
@discardableResult public func run(_ bindings: Binding?...) throws -> Statement { return Statement(connection, "") }
34-
@discardableResult public func run(_ bindings: [Binding?]) throws -> Statement { return Statement(connection, "") }
35-
@discardableResult public func run(_ bindings: [String: Binding?]) throws -> Statement { return Statement(connection, "") }
33+
@discardableResult public func run(_ bindings: Binding?...) throws -> Statement { return self }
34+
@discardableResult public func run(_ bindings: [Binding?]) throws -> Statement { return self }
35+
@discardableResult public func run(_ bindings: [String: Binding?]) throws -> Statement { return self }
3636

3737
public func scalar(_ bindings: Binding?...) throws -> Binding? { return nil }
3838
public func scalar(_ bindings: [Binding?]) throws -> Binding? { return nil }
@@ -42,13 +42,13 @@ class Statement {
4242
class Connection {
4343
public func execute(_ SQL: String) throws { }
4444

45-
public func prepare(_ statement: String, _ bindings: Binding?...) throws -> Statement { return Statement(self, "") }
46-
public func prepare(_ statement: String, _ bindings: [Binding?]) throws -> Statement { return Statement(self, "") }
47-
public func prepare(_ statement: String, _ bindings: [String: Binding?]) throws -> Statement { return Statement(self, "") }
45+
public func prepare(_ statement: String, _ bindings: Binding?...) throws -> Statement { return try Statement(self, "") }
46+
public func prepare(_ statement: String, _ bindings: [Binding?]) throws -> Statement { return try Statement(self, "") }
47+
public func prepare(_ statement: String, _ bindings: [String: Binding?]) throws -> Statement { return try Statement(self, "") }
4848

49-
@discardableResult public func run(_ statement: String, _ bindings: Binding?...) throws -> Statement { return Statement(self, "") }
50-
@discardableResult public func run(_ statement: String, _ bindings: [Binding?]) throws -> Statement { return Statement(self, "") }
51-
@discardableResult public func run(_ statement: String, _ bindings: [String: Binding?]) throws -> Statement { return Statement(self, "") }
49+
@discardableResult public func run(_ statement: String, _ bindings: Binding?...) throws -> Statement { return try Statement(self, "") }
50+
@discardableResult public func run(_ statement: String, _ bindings: [Binding?]) throws -> Statement { return try Statement(self, "") }
51+
@discardableResult public func run(_ statement: String, _ bindings: [String: Binding?]) throws -> Statement { return try Statement(self, "") }
5252

5353
public func scalar(_ statement: String, _ bindings: Binding?...) throws -> Binding? { return nil }
5454
public func scalar(_ statement: String, _ bindings: [Binding?]) throws -> Binding? { return nil }
@@ -57,9 +57,9 @@ class Connection {
5757

5858
// --- tests ---
5959

60-
func test_sqlite_swift_api(db: Connection) {
60+
func test_sqlite_swift_api(db: Connection) throws {
6161
let localString = "user"
62-
let remoteString = try! String(contentsOf: URL(string: "http://example.com/")!)
62+
let remoteString = try String(contentsOf: URL(string: "http://example.com/")!)
6363
let remoteNumber = Int(remoteString) ?? 0
6464

6565
let unsafeQuery1 = remoteString
@@ -89,11 +89,11 @@ func test_sqlite_swift_api(db: Connection) {
8989
let stmt3 = try db.prepare(varQuery, remoteString) // GOOD
9090
try stmt3.run()
9191

92-
let stmt4 = Statement(db, localString) // GOOD
93-
stmt4.run()
92+
let stmt4 = try Statement(db, localString) // GOOD
93+
try stmt4.run()
9494

95-
let stmt5 = Statement(db, remoteString) // BAD
96-
stmt5.run()
95+
let stmt5 = try Statement(db, remoteString) // BAD
96+
try stmt5.run()
9797

9898
// --- more variants ---
9999

@@ -106,28 +106,28 @@ func test_sqlite_swift_api(db: Connection) {
106106
let stmt8 = try db.prepare(unsafeQuery1, ["username": ""]) // BAD
107107
try stmt8.run()
108108

109-
db.run(unsafeQuery1, "") // BAD
109+
try db.run(unsafeQuery1, "") // BAD
110110

111-
db.run(unsafeQuery1, [""]) // BAD
111+
try db.run(unsafeQuery1, [""]) // BAD
112112

113-
db.run(unsafeQuery1, ["username": ""]) // BAD
113+
try db.run(unsafeQuery1, ["username": ""]) // BAD
114114

115-
db.scalar(unsafeQuery1, "") // BAD
115+
try db.scalar(unsafeQuery1, "") // BAD
116116

117-
db.scalar(unsafeQuery1, [""]) // BAD
117+
try db.scalar(unsafeQuery1, [""]) // BAD
118118

119-
db.scalar(unsafeQuery1, ["username": ""]) // BAD
119+
try db.scalar(unsafeQuery1, ["username": ""]) // BAD
120120

121121
let stmt9 = try db.prepare(varQuery) // GOOD
122-
stmt9.bind(remoteString) // GOOD
123-
stmt9.bind([remoteString]) // GOOD
124-
stmt9.bind(["username": remoteString]) // GOOD
122+
try stmt9.bind(remoteString) // GOOD
123+
try stmt9.bind([remoteString]) // GOOD
124+
try stmt9.bind(["username": remoteString]) // GOOD
125125
try stmt9.run(remoteString) // GOOD
126126
try stmt9.run([remoteString]) // GOOD
127127
try stmt9.run(["username": remoteString]) // GOOD
128128
try stmt9.scalar(remoteString) // GOOD
129129
try stmt9.scalar([remoteString]) // GOOD
130130
try stmt9.scalar(["username": remoteString]) // GOOD
131131

132-
Statement(db, remoteString).run() // BAD
132+
try Statement(db, remoteString).run() // BAD
133133
}

0 commit comments

Comments
 (0)