From 903b0f5bab5b6dbf30eb87c9573af3770d0fbc54 Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Tue, 19 Sep 2023 13:31:48 +0100 Subject: [PATCH] Swift: Add numeric barrier for the SQL Injinjection query. --- .../swift/security/SqlInjectionExtensions.qll | 10 ++++++++++ .../test/query-tests/Security/CWE-089/SQLite.swift | 2 +- .../Security/CWE-089/SqlInjection.expected | 14 -------------- .../Security/CWE-089/sqlite3_c_api.swift | 2 +- 4 files changed, 12 insertions(+), 16 deletions(-) diff --git a/swift/ql/lib/codeql/swift/security/SqlInjectionExtensions.qll b/swift/ql/lib/codeql/swift/security/SqlInjectionExtensions.qll index 1aac3571f53..48b2cadb3a8 100644 --- a/swift/ql/lib/codeql/swift/security/SqlInjectionExtensions.qll +++ b/swift/ql/lib/codeql/swift/security/SqlInjectionExtensions.qll @@ -153,3 +153,13 @@ private class GrdbDefaultSqlInjectionSink extends SqlInjectionSink { private class DefaultSqlInjectionSink extends SqlInjectionSink { DefaultSqlInjectionSink() { sinkNode(this, "sql-injection") } } + +/** + * A barrier for SQL injection. + */ +private class SqlInjectionDefaultBarrier extends SqlInjectionBarrier { + SqlInjectionDefaultBarrier() { + // any numeric type + this.asExpr().getType().getUnderlyingType().getABaseType*().getName() = "Numeric" + } +} diff --git a/swift/ql/test/query-tests/Security/CWE-089/SQLite.swift b/swift/ql/test/query-tests/Security/CWE-089/SQLite.swift index 7746c02a9d5..f9a6b41340c 100644 --- a/swift/ql/test/query-tests/Security/CWE-089/SQLite.swift +++ b/swift/ql/test/query-tests/Security/CWE-089/SQLite.swift @@ -74,7 +74,7 @@ func test_sqlite_swift_api(db: Connection) throws { try db.execute(unsafeQuery2) // BAD try db.execute(unsafeQuery3) // BAD try db.execute(safeQuery1) // GOOD - try db.execute(safeQuery2) // GOOD [FALSE POSITIVE] + try db.execute(safeQuery2) // GOOD // --- prepared statements --- diff --git a/swift/ql/test/query-tests/Security/CWE-089/SqlInjection.expected b/swift/ql/test/query-tests/Security/CWE-089/SqlInjection.expected index 65b3383cdef..a4617e9b811 100644 --- a/swift/ql/test/query-tests/Security/CWE-089/SqlInjection.expected +++ b/swift/ql/test/query-tests/Security/CWE-089/SqlInjection.expected @@ -82,7 +82,6 @@ edges | GRDB.swift:342:26:342:80 | call to String.init(contentsOf:) | GRDB.swift:349:84:349:84 | remoteString | | GRDB.swift:342:26:342:80 | call to String.init(contentsOf:) | GRDB.swift:350:69:350:69 | remoteString | | GRDB.swift:342:26:342:80 | call to String.init(contentsOf:) | GRDB.swift:351:84:351:84 | remoteString | -| SQLite.swift:62:25:62:79 | call to String.init(contentsOf:) | SQLite.swift:63:25:63:25 | remoteString | | SQLite.swift:62:25:62:79 | call to String.init(contentsOf:) | SQLite.swift:73:17:73:17 | unsafeQuery1 | | SQLite.swift:62:25:62:79 | call to String.init(contentsOf:) | SQLite.swift:74:17:74:17 | unsafeQuery2 | | SQLite.swift:62:25:62:79 | call to String.init(contentsOf:) | SQLite.swift:75:17:75:17 | unsafeQuery3 | @@ -98,9 +97,6 @@ edges | SQLite.swift:62:25:62:79 | call to String.init(contentsOf:) | SQLite.swift:117:16:117:16 | unsafeQuery1 | | SQLite.swift:62:25:62:79 | call to String.init(contentsOf:) | SQLite.swift:119:16:119:16 | unsafeQuery1 | | SQLite.swift:62:25:62:79 | call to String.init(contentsOf:) | SQLite.swift:132:20:132:20 | remoteString | -| SQLite.swift:63:21:63:37 | call to Self.init(_:) | SQLite.swift:77:17:77:17 | safeQuery2 | -| SQLite.swift:63:25:63:25 | remoteString | SQLite.swift:63:21:63:37 | call to Self.init(_:) | -| sqlite3_c_api.swift:122:26:122:80 | call to String.init(contentsOf:) | sqlite3_c_api.swift:123:25:123:25 | remoteString | | sqlite3_c_api.swift:122:26:122:80 | call to String.init(contentsOf:) | sqlite3_c_api.swift:133:33:133:33 | unsafeQuery1 | | sqlite3_c_api.swift:122:26:122:80 | call to String.init(contentsOf:) | sqlite3_c_api.swift:134:33:134:33 | unsafeQuery2 | | sqlite3_c_api.swift:122:26:122:80 | call to String.init(contentsOf:) | sqlite3_c_api.swift:135:33:135:33 | unsafeQuery3 | @@ -108,8 +104,6 @@ edges | sqlite3_c_api.swift:122:26:122:80 | call to String.init(contentsOf:) | sqlite3_c_api.swift:175:29:175:29 | unsafeQuery3 | | sqlite3_c_api.swift:122:26:122:80 | call to String.init(contentsOf:) | sqlite3_c_api.swift:183:29:183:29 | unsafeQuery3 | | sqlite3_c_api.swift:122:26:122:80 | call to String.init(contentsOf:) | sqlite3_c_api.swift:189:13:189:13 | unsafeQuery3 | -| sqlite3_c_api.swift:123:21:123:37 | call to Self.init(_:) | sqlite3_c_api.swift:137:33:137:33 | safeQuery2 | -| sqlite3_c_api.swift:123:25:123:25 | remoteString | sqlite3_c_api.swift:123:21:123:37 | call to Self.init(_:) | | sqlite3_c_api.swift:189:13:189:13 | unsafeQuery3 | sqlite3_c_api.swift:189:13:189:58 | call to data(using:allowLossyConversion:) | | sqlite3_c_api.swift:189:13:189:58 | call to data(using:allowLossyConversion:) | sqlite3_c_api.swift:190:2:190:2 | data | | sqlite3_c_api.swift:190:2:190:2 | data | sqlite3_c_api.swift:190:21:190:21 | [post] buffer | @@ -213,12 +207,9 @@ nodes | GRDB.swift:350:69:350:69 | remoteString | semmle.label | remoteString | | GRDB.swift:351:84:351:84 | remoteString | semmle.label | remoteString | | SQLite.swift:62:25:62:79 | call to String.init(contentsOf:) | semmle.label | call to String.init(contentsOf:) | -| SQLite.swift:63:21:63:37 | call to Self.init(_:) | semmle.label | call to Self.init(_:) | -| SQLite.swift:63:25:63:25 | remoteString | semmle.label | remoteString | | SQLite.swift:73:17:73:17 | unsafeQuery1 | semmle.label | unsafeQuery1 | | SQLite.swift:74:17:74:17 | unsafeQuery2 | semmle.label | unsafeQuery2 | | SQLite.swift:75:17:75:17 | unsafeQuery3 | semmle.label | unsafeQuery3 | -| SQLite.swift:77:17:77:17 | safeQuery2 | semmle.label | safeQuery2 | | SQLite.swift:83:29:83:29 | unsafeQuery3 | semmle.label | unsafeQuery3 | | SQLite.swift:95:32:95:32 | remoteString | semmle.label | remoteString | | SQLite.swift:100:29:100:29 | unsafeQuery1 | semmle.label | unsafeQuery1 | @@ -232,12 +223,9 @@ nodes | SQLite.swift:119:16:119:16 | unsafeQuery1 | semmle.label | unsafeQuery1 | | SQLite.swift:132:20:132:20 | remoteString | semmle.label | remoteString | | sqlite3_c_api.swift:122:26:122:80 | call to String.init(contentsOf:) | semmle.label | call to String.init(contentsOf:) | -| sqlite3_c_api.swift:123:21:123:37 | call to Self.init(_:) | semmle.label | call to Self.init(_:) | -| sqlite3_c_api.swift:123:25:123:25 | remoteString | semmle.label | remoteString | | sqlite3_c_api.swift:133:33:133:33 | unsafeQuery1 | semmle.label | unsafeQuery1 | | sqlite3_c_api.swift:134:33:134:33 | unsafeQuery2 | semmle.label | unsafeQuery2 | | sqlite3_c_api.swift:135:33:135:33 | unsafeQuery3 | semmle.label | unsafeQuery3 | -| sqlite3_c_api.swift:137:33:137:33 | safeQuery2 | semmle.label | safeQuery2 | | sqlite3_c_api.swift:145:26:145:26 | unsafeQuery3 | semmle.label | unsafeQuery3 | | sqlite3_c_api.swift:175:29:175:29 | unsafeQuery3 | semmle.label | unsafeQuery3 | | sqlite3_c_api.swift:183:29:183:29 | unsafeQuery3 | semmle.label | unsafeQuery3 | @@ -336,7 +324,6 @@ subpaths | SQLite.swift:73:17:73:17 | unsafeQuery1 | SQLite.swift:62:25:62:79 | call to String.init(contentsOf:) | SQLite.swift:73:17:73:17 | unsafeQuery1 | This query depends on a $@. | SQLite.swift:62:25:62:79 | call to String.init(contentsOf:) | user-provided value | | SQLite.swift:74:17:74:17 | unsafeQuery2 | SQLite.swift:62:25:62:79 | call to String.init(contentsOf:) | SQLite.swift:74:17:74:17 | unsafeQuery2 | This query depends on a $@. | SQLite.swift:62:25:62:79 | call to String.init(contentsOf:) | user-provided value | | SQLite.swift:75:17:75:17 | unsafeQuery3 | SQLite.swift:62:25:62:79 | call to String.init(contentsOf:) | SQLite.swift:75:17:75:17 | unsafeQuery3 | This query depends on a $@. | SQLite.swift:62:25:62:79 | call to String.init(contentsOf:) | user-provided value | -| SQLite.swift:77:17:77:17 | safeQuery2 | SQLite.swift:62:25:62:79 | call to String.init(contentsOf:) | SQLite.swift:77:17:77:17 | safeQuery2 | This query depends on a $@. | SQLite.swift:62:25:62:79 | call to String.init(contentsOf:) | user-provided value | | SQLite.swift:83:29:83:29 | unsafeQuery3 | SQLite.swift:62:25:62:79 | call to String.init(contentsOf:) | SQLite.swift:83:29:83:29 | unsafeQuery3 | This query depends on a $@. | SQLite.swift:62:25:62:79 | call to String.init(contentsOf:) | user-provided value | | SQLite.swift:95:32:95:32 | remoteString | SQLite.swift:62:25:62:79 | call to String.init(contentsOf:) | SQLite.swift:95:32:95:32 | remoteString | This query depends on a $@. | SQLite.swift:62:25:62:79 | call to String.init(contentsOf:) | user-provided value | | SQLite.swift:100:29:100:29 | unsafeQuery1 | SQLite.swift:62:25:62:79 | call to String.init(contentsOf:) | SQLite.swift:100:29:100:29 | unsafeQuery1 | This query depends on a $@. | SQLite.swift:62:25:62:79 | call to String.init(contentsOf:) | user-provided value | @@ -352,7 +339,6 @@ subpaths | sqlite3_c_api.swift:133:33:133:33 | unsafeQuery1 | sqlite3_c_api.swift:122:26:122:80 | call to String.init(contentsOf:) | sqlite3_c_api.swift:133:33:133:33 | unsafeQuery1 | This query depends on a $@. | sqlite3_c_api.swift:122:26:122:80 | call to String.init(contentsOf:) | user-provided value | | sqlite3_c_api.swift:134:33:134:33 | unsafeQuery2 | sqlite3_c_api.swift:122:26:122:80 | call to String.init(contentsOf:) | sqlite3_c_api.swift:134:33:134:33 | unsafeQuery2 | This query depends on a $@. | sqlite3_c_api.swift:122:26:122:80 | call to String.init(contentsOf:) | user-provided value | | sqlite3_c_api.swift:135:33:135:33 | unsafeQuery3 | sqlite3_c_api.swift:122:26:122:80 | call to String.init(contentsOf:) | sqlite3_c_api.swift:135:33:135:33 | unsafeQuery3 | This query depends on a $@. | sqlite3_c_api.swift:122:26:122:80 | call to String.init(contentsOf:) | user-provided value | -| sqlite3_c_api.swift:137:33:137:33 | safeQuery2 | sqlite3_c_api.swift:122:26:122:80 | call to String.init(contentsOf:) | sqlite3_c_api.swift:137:33:137:33 | safeQuery2 | This query depends on a $@. | sqlite3_c_api.swift:122:26:122:80 | call to String.init(contentsOf:) | user-provided value | | sqlite3_c_api.swift:145:26:145:26 | unsafeQuery3 | sqlite3_c_api.swift:122:26:122:80 | call to String.init(contentsOf:) | sqlite3_c_api.swift:145:26:145:26 | unsafeQuery3 | This query depends on a $@. | sqlite3_c_api.swift:122:26:122:80 | call to String.init(contentsOf:) | user-provided value | | sqlite3_c_api.swift:175:29:175:29 | unsafeQuery3 | sqlite3_c_api.swift:122:26:122:80 | call to String.init(contentsOf:) | sqlite3_c_api.swift:175:29:175:29 | unsafeQuery3 | This query depends on a $@. | sqlite3_c_api.swift:122:26:122:80 | call to String.init(contentsOf:) | user-provided value | | sqlite3_c_api.swift:183:29:183:29 | unsafeQuery3 | sqlite3_c_api.swift:122:26:122:80 | call to String.init(contentsOf:) | sqlite3_c_api.swift:183:29:183:29 | unsafeQuery3 | This query depends on a $@. | sqlite3_c_api.swift:122:26:122:80 | call to String.init(contentsOf:) | user-provided value | diff --git a/swift/ql/test/query-tests/Security/CWE-089/sqlite3_c_api.swift b/swift/ql/test/query-tests/Security/CWE-089/sqlite3_c_api.swift index 06846cd2072..8498d89d68d 100644 --- a/swift/ql/test/query-tests/Security/CWE-089/sqlite3_c_api.swift +++ b/swift/ql/test/query-tests/Security/CWE-089/sqlite3_c_api.swift @@ -134,7 +134,7 @@ func test_sqlite3_c_api(db: OpaquePointer?, buffer: UnsafeMutablePointer) let result2 = sqlite3_exec(db, unsafeQuery2, nil, nil, nil) // BAD let result3 = sqlite3_exec(db, unsafeQuery3, nil, nil, nil) // BAD let result4 = sqlite3_exec(db, safeQuery1, nil, nil, nil) // GOOD - let result5 = sqlite3_exec(db, safeQuery2, nil, nil, nil) // GOOD [FALSE POSITIVE] + let result5 = sqlite3_exec(db, safeQuery2, nil, nil, nil) // GOOD // --- prepared statements ---