Skip to content

Swift: Add sinks for sqlite3 and SQLite.swift to swift/cleartext-storage-database #14312

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rdmarsh2 merged 11 commits into from
Oct 5, 2023
Merged

Swift: Add sinks for sqlite3 and SQLite.swift to swift/cleartext-storage-database #14312

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
4245a38
Swift: Add SQLite.swift and sqlite3 C API test cases for swift/cleart…
geoffw0 May 11, 2023
16ae637
Swift: Add sqlite3 models.
geoffw0 Sep 21, 2023
839b963
Swift: Effect of fixing string interpolation bug.
geoffw0 Sep 21, 2023
4350060
Swift: Add SQLite.swift models.
geoffw0 Sep 21, 2023
6be01ea
Swift: Add implict read steps for dictionary content.
geoffw0 Sep 25, 2023
51ed824
Swift: Add more SQLite.swift models.
geoffw0 Sep 21, 2023
f9a617c
Swift: Change note.
geoffw0 Sep 21, 2023
0f1711f
Swift: Test insertMany.
geoffw0 Oct 2, 2023
6bea7f8
Merge branch 'main' into sqlpathinject2
geoffw0 Oct 5, 2023
7ddece1
Swift: Update .expected after merge.
geoffw0 Oct 5, 2023
fdcc6b4
Swift: Simplify allowImplicitRead slightly.
geoffw0 Oct 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions swift/ql/lib/codeql/swift/frameworks/Frameworks.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

private import Alamofire.Alamofire
private import JavaScriptCore.JavaScriptCore
private import SQL.SQL
private import StandardLibrary.StandardLibrary
private import UIKit.UIKit
private import Xml.Xml
23 changes: 23 additions & 0 deletions swift/ql/lib/codeql/swift/frameworks/SQL/SQL.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
/**
* Provides models for SQL libraries.
*/

import swift
private import codeql.swift.dataflow.ExternalFlow

/**
* A model for SQL library functions that permit taint flow.
*/
private class FilePathSummaries extends SummaryModelCsv {
override predicate row(string row) {
row =
[
// SQLite.Swift
";;false;<-(_:_:);;;Argument[0..1];ReturnValue;taint",
";Expression;true;init(_:_:);;;Argument[0];ReturnValue;taint",
";Expression;true;init(_:_:);;;Argument[1].CollectionElement;ReturnValue;taint",
";ExpressionType;true;init(_:);;;Argument[0];ReturnValue;taint",
";ExpressionType;true;replace(_:with:);;;Argument[1];ReturnValue;taint",
]
}
}
43 changes: 42 additions & 1 deletion swift/ql/lib/codeql/swift/security/CleartextStorageDatabaseExtensions.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,48 @@ private class CleartextStorageDatabaseSinks extends SinkModelCsv {
";FetchableRecord;true;fetchOne(_:arguments:adapter:);;;Argument[1];database-store",
";Statement;true;execute(arguments:);;;Argument[0];database-store",
";CommonTableExpression;true;init(recursive:named:columns:sql:arguments:);;;Argument[4];database-store",
";Statement;true;setArguments(_:);;;Argument[0];database-store"
";Statement;true;setArguments(_:);;;Argument[0];database-store",
// sqlite3 sinks
";;false;sqlite3_exec(_:_:_:_:_:);;;Argument[1];database-store",
";;false;sqlite3_prepare(_:_:_:_:_:);;;Argument[1];database-store",
";;false;sqlite3_prepare_v2(_:_:_:_:_:);;;Argument[1];database-store",
";;false;sqlite3_prepare_v3(_:_:_:_:_:_:);;;Argument[1];database-store",
";;false;sqlite3_prepare16(_:_:_:_:_:);;;Argument[1];database-store",
";;false;sqlite3_prepare16_v2(_:_:_:_:_:);;;Argument[1];database-store",
";;false;sqlite3_prepare16_v3(_:_:_:_:_:);;;Argument[1];database-store",
";;false;sqlite3_bind_blob(_:_:_:_:_:);;;Argument[2];database-store",
";;false;sqlite3_bind_blob64(_:_:_:_:_:);;;Argument[2];database-store",
";;false;sqlite3_bind_double(_:_:_:);;;Argument[2];database-store",
";;false;sqlite3_bind_int(_:_:_:);;;Argument[2];database-store",
";;false;sqlite3_bind_int64(_:_:_:);;;Argument[2];database-store",
";;false;sqlite3_bind_text(_:_:_:_:_:);;;Argument[2];database-store",
";;false;sqlite3_bind_text16(_:_:_:_:_:);;;Argument[2];database-store",
";;false;sqlite3_bind_text64(_:_:_:_:_:_:);;;Argument[2];database-store",
";;false;sqlite3_bind_value(_:_:_:);;;Argument[2];database-store",
";;false;sqlite3_bind_pointer(_:_:_:_:);;;Argument[2];database-store",
// SQLite.swift
";Connection;true;execute(_:);;;Argument[0];database-store",
";Connection;true;prepare(_:_:);;;Argument[0];database-store",
";Connection;true;prepare(_:_:);;;Argument[1];database-store",
";Connection;true;run(_:_:);;;Argument[0];database-store",
";Connection;true;run(_:_:);;;Argument[1];database-store",
";Connection;true;scalar(_:_:);;;Argument[0];database-store",
";Connection;true;scalar(_:_:);;;Argument[1];database-store",
";Statement;true;init(_:_:);;;Argument[1];database-store",
";Statement;true;bind(_:);;;Argument[0];database-store",
";Statement;true;run(_:);;;Argument[0];database-store",
";Statement;true;scalar(_:);;;Argument[0];database-store",
";QueryType;true;insert(_:);;;Argument[0];database-store",
";QueryType;true;insert(_:_:);;;Argument[0..1];database-store",
";QueryType;true;insert(or:_:);;;Argument[1];database-store",
";QueryType;true;insertMany(_:);;;Argument[0];database-store",
Copy link
Copy Markdown
Contributor

@rdmarsh2 rdmarsh2 Sep 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be Argument[0].CollectionElement?

Copy link
Copy Markdown
Contributor Author

@geoffw0 geoffw0 Oct 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but it can't be because at present MAD doesn't support access path syntax on sinks (or sources). I'm pushing for this feature because I think it's the natural way to express sinks like this.

The case should covered by the additional taint step at present, and that will be upgraded to an implicit read step in this query in #14357 .

I've just added tests for insertMany to this PR, as it's a special case that clearly deserves a test of its own.

";QueryType;true;insertMany(or:_:);;;Argument[1];database-store",
";QueryType;true;upsert(_:onConflictOf:);;;Argument[0];database-store",
";QueryType;true;upsert(_:onConflictOf:setValues:);;;Argument[0];database-store",
";QueryType;true;upsert(_:onConflictOf:setValues:);;;Argument[2];database-store",
";QueryType;true;update(_:);;;Argument[0];database-store",
";QueryType;true;update(_:_:);;;Argument[0..1];database-store",
";QueryType;true;update(or:_:);;;Argument[1];database-store",
]
}
}
Expand Down
8 changes: 7 additions & 1 deletion swift/ql/lib/codeql/swift/security/CleartextStorageDatabaseQuery.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,13 @@ module CleartextStorageDatabaseConfig implements DataFlow::ConfigSig {
c.getAReadContent().(DataFlow::Content::FieldContent).getField() = cx.getAMember()
)
or
// flow out from array elements of at the sink,
// flow out from dictionary tuple values at the sink (this is essential
// for some of the SQLite.swift models).
isSink(node) and
node.asExpr().getType().getUnderlyingType() instanceof DictionaryType and
c.getAReadContent().(DataFlow::Content::TupleContent).getIndex() = 1
or
// flow out from array elements (and other collection content) at the sink,
// for example in `database.allStatements(sql: "", arguments: [sensitive])`.
isSink(node) and
c.getAReadContent() instanceof DataFlow::Content::CollectionContent
Expand Down
4 changes: 4 additions & 0 deletions swift/ql/src/change-notes/2023-09-22-cleartext-storage-database-sinks.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* Added sqlite3 and SQLite.swift sinks and flow summaries for the `swift/cleartext-storage-database` query.
Loading