From 9f6a5d9e13fcc459fd074567fe9d662a50644883 Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Tue, 30 Jul 2024 17:00:40 +0100 Subject: [PATCH 1/5] Swift: Fix typo in example. --- swift/ql/src/queries/Security/CWE-089/SqlInjectionGood.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/swift/ql/src/queries/Security/CWE-089/SqlInjectionGood.swift b/swift/ql/src/queries/Security/CWE-089/SqlInjectionGood.swift index 8db8a9367b2..c50d0cd196e 100644 --- a/swift/ql/src/queries/Security/CWE-089/SqlInjectionGood.swift +++ b/swift/ql/src/queries/Security/CWE-089/SqlInjectionGood.swift @@ -1,4 +1,4 @@ let safeQuery = "SELECT * FROM users WHERE username=?" let stmt = try db.prepare(safeQuery, userControlledString) // GOOD -try stmt2.run() \ No newline at end of file +try stmt.run() From 2fd4b57d746c5aea6f6f4b9c64cc18be441f02bf Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Tue, 30 Jul 2024 22:39:14 +0100 Subject: [PATCH 2/5] Swift: Expand the swift/sql-injection qhelp examples by labelling the API that's used, adding SQLite3 C API examples, and adding an example of using a prepared statement incorrectly. --- .../queries/Security/CWE-089/SqlInjection.qhelp | 4 ++-- .../Security/CWE-089/SqlInjectionBad.swift | 13 +++++++++++-- .../Security/CWE-089/SqlInjectionGood.swift | 15 +++++++++++++++ 3 files changed, 28 insertions(+), 4 deletions(-) diff --git a/swift/ql/src/queries/Security/CWE-089/SqlInjection.qhelp b/swift/ql/src/queries/Security/CWE-089/SqlInjection.qhelp index 808d5052ea5..64927678eb3 100644 --- a/swift/ql/src/queries/Security/CWE-089/SqlInjection.qhelp +++ b/swift/ql/src/queries/Security/CWE-089/SqlInjection.qhelp @@ -18,7 +18,7 @@ Most database connector libraries offer a way to safely embed untrusted data int -

In the following example, a SQL query is prepared using string interpolation to directly include a user-controlled value userControlledString in the query. An attacker could craft userControlledString to change the overall meaning of the SQL query. +

In the following examples, an SQL query is prepared using string interpolation to directly include a user-controlled value userControlledString in the query. An attacker could craft userControlledString to change the overall meaning of the SQL query.

@@ -35,4 +35,4 @@ Most database connector libraries offer a way to safely embed untrusted data int
  • OWASP: SQL Injection Prevention Cheat Sheet.
    • - \ No newline at end of file + diff --git a/swift/ql/src/queries/Security/CWE-089/SqlInjectionBad.swift b/swift/ql/src/queries/Security/CWE-089/SqlInjectionBad.swift index 0b5c250a67b..085b1b54798 100644 --- a/swift/ql/src/queries/Security/CWE-089/SqlInjectionBad.swift +++ b/swift/ql/src/queries/Security/CWE-089/SqlInjectionBad.swift @@ -1,3 +1,12 @@ -let unsafeQuery = "SELECT * FROM users WHERE username='\(userControlledString)'" // BAD +// with SQLite.swift -try db.execute(unsafeQuery) \ No newline at end of file +let unsafeQuery = "SELECT * FROM users WHERE username='\(userControlledString)'" + +try db.execute(unsafeQuery) // BAD + +let stmt = try db.prepare(unsafeQuery) // also BAD +try stmt.run() + +// with SQLite3 C API + +let result = sqlite3_exec(db, unsafeQuery, nil, nil, nil) // BAD diff --git a/swift/ql/src/queries/Security/CWE-089/SqlInjectionGood.swift b/swift/ql/src/queries/Security/CWE-089/SqlInjectionGood.swift index c50d0cd196e..8d4228fca50 100644 --- a/swift/ql/src/queries/Security/CWE-089/SqlInjectionGood.swift +++ b/swift/ql/src/queries/Security/CWE-089/SqlInjectionGood.swift @@ -1,4 +1,19 @@ +// with SQLite.swift + let safeQuery = "SELECT * FROM users WHERE username=?" let stmt = try db.prepare(safeQuery, userControlledString) // GOOD try stmt.run() + +// with sqlite3 C API + +var stmt2: OpaquePointer? + +if (sqlite3_prepare_v2(db, safeQuery, -1, &stmt2, nil) == SQLITE_OK) { + if (sqlite3_bind_text(stmt2, 1, userControlledString, -1, SQLITE_TRANSIENT) == SQLITE_OK) { // GOOD + let result = sqlite3_step(stmt2) + + // ... + } + sqlite3_finalize(stmt2) +} From 2ed2a768662f349e2634c33021cb6d25c2bd1c5b Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Tue, 30 Jul 2024 23:37:26 +0100 Subject: [PATCH 3/5] Swift: Add a note about escaping as an alternative way to fix these issues. --- swift/ql/src/queries/Security/CWE-089/SqlInjection.qhelp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/swift/ql/src/queries/Security/CWE-089/SqlInjection.qhelp b/swift/ql/src/queries/Security/CWE-089/SqlInjection.qhelp index 64927678eb3..0ca1764e86f 100644 --- a/swift/ql/src/queries/Security/CWE-089/SqlInjection.qhelp +++ b/swift/ql/src/queries/Security/CWE-089/SqlInjection.qhelp @@ -12,7 +12,7 @@ If a database query (such as a SQL query) is built from user-provided data witho

    -Most database connector libraries offer a way to safely embed untrusted data into a query using query parameters or prepared statements. You should use these features to build queries, rather than string concatenation or similar methods without sufficient sanitization. +Most database connector libraries offer a way to safely embed untrusted data into a query using query parameters or prepared statements. You should use these features to build queries, rather than string concatenation or similar methods. It's also possible to escape (sanitize) user-controlled strings so that they can be included directly in an SQL command, but this approach is only safe if the chosen escaping function is robust.

    From fa898b848945deb68f3abb689912b385c4990155 Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Fri, 2 Aug 2024 13:11:01 +0100 Subject: [PATCH 4/5] Update swift/ql/src/queries/Security/CWE-089/SqlInjection.qhelp Co-authored-by: Ben Ahmady <32935794+subatoi@users.noreply.github.com> --- swift/ql/src/queries/Security/CWE-089/SqlInjection.qhelp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/swift/ql/src/queries/Security/CWE-089/SqlInjection.qhelp b/swift/ql/src/queries/Security/CWE-089/SqlInjection.qhelp index 0ca1764e86f..9dfe6cf0862 100644 --- a/swift/ql/src/queries/Security/CWE-089/SqlInjection.qhelp +++ b/swift/ql/src/queries/Security/CWE-089/SqlInjection.qhelp @@ -12,7 +12,7 @@ If a database query (such as a SQL query) is built from user-provided data witho

    -Most database connector libraries offer a way to safely embed untrusted data into a query using query parameters or prepared statements. You should use these features to build queries, rather than string concatenation or similar methods. It's also possible to escape (sanitize) user-controlled strings so that they can be included directly in an SQL command, but this approach is only safe if the chosen escaping function is robust. +Most database connector libraries offer a way to safely embed untrusted data into a query using query parameters or prepared statements. You should use these features to build queries, rather than string concatenation or similar methods. You can also escape (sanitize) user-controlled strings so that they can be included directly in an SQL command, but this approach is only safe if the chosen escaping function is robust.

    From e66cd05f9684cb6f64f4a9f96a6792f0b527180f Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Fri, 2 Aug 2024 13:31:03 +0100 Subject: [PATCH 5/5] Swift: Improve phrasing around robust escape functions. --- swift/ql/src/queries/Security/CWE-089/SqlInjection.qhelp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/swift/ql/src/queries/Security/CWE-089/SqlInjection.qhelp b/swift/ql/src/queries/Security/CWE-089/SqlInjection.qhelp index 9dfe6cf0862..6975d95d48a 100644 --- a/swift/ql/src/queries/Security/CWE-089/SqlInjection.qhelp +++ b/swift/ql/src/queries/Security/CWE-089/SqlInjection.qhelp @@ -12,7 +12,7 @@ If a database query (such as a SQL query) is built from user-provided data witho

    -Most database connector libraries offer a way to safely embed untrusted data into a query using query parameters or prepared statements. You should use these features to build queries, rather than string concatenation or similar methods. You can also escape (sanitize) user-controlled strings so that they can be included directly in an SQL command, but this approach is only safe if the chosen escaping function is robust. +Most database connector libraries offer a way to safely embed untrusted data into a query using query parameters or prepared statements. You should use these features to build queries, rather than string concatenation or similar methods. You can also escape (sanitize) user-controlled strings so that they can be included directly in an SQL command. A library function should be used for escaping, because this approach is only safe if the escaping function is robust against all possible inputs.