Skip to content

Commit 0d7769f

Browse files
authored
Merge pull request github#13276 from geoffw0/sqlpathinject
Swift: Add path injection sinks for sqlite3 and SQLite.swift
2 parents 596f701 + 3fd5de8 commit 0d7769f

File tree

7 files changed

+246
-89
lines changed

7 files changed

+246
-89
lines changed
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,37 @@
11
private import codeql.swift.generated.decl.EnumElementDecl
2+
private import codeql.swift.elements.decl.EnumDecl
23

4+
/**
5+
* An enum element declaration, for example `enumElement` and `anotherEnumElement` in:
6+
* ```
7+
* enum MyEnum {
8+
* case enumElement
9+
* case anotherEnumElement(Int)
10+
* }
11+
* ```
12+
*/
313
class EnumElementDecl extends Generated::EnumElementDecl {
414
override string toString() { result = this.getName() }
15+
16+
/**
17+
* Holds if this enum element declaration is called `enumElementName` and is a member of an
18+
* enum called `enumName`.
19+
*/
20+
cached
21+
predicate hasQualifiedName(string enumName, string enumElementName) {
22+
this.getName() = enumElementName and
23+
exists(EnumDecl d |
24+
d.getFullName() = enumName and
25+
d.getAMember() = this
26+
)
27+
}
28+
29+
/**
30+
* Holds if this enum element declaration is called `enumElementName` and is a member of an
31+
* enumcalled `enumName` in a module called `moduleName`.
32+
*/
33+
predicate hasQualifiedName(string moduleName, string enumName, string enumElementName) {
34+
this.hasQualifiedName(enumName, enumElementName) and
35+
this.getModule().getFullName() = moduleName
36+
}
537
}

swift/ql/lib/codeql/swift/elements/expr/EnumElementExpr.qll

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,14 @@ private import codeql.swift.elements.expr.MethodLookupExpr
1010
private import codeql.swift.elements.decl.EnumElementDecl
1111

1212
/**
13-
* An expression that constructs a case of an enum.
13+
* An expression that references a case of an enum. For example both `enumElement` in:
14+
* ```
15+
* let value = MyEnum.enumElement
16+
* ...
17+
* switch (anotherValue) {
18+
* case .enumElement:
19+
* ...
20+
* ```
1421
*/
1522
class EnumElementExpr extends Expr {
1623
EnumElementDecl decl;

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,9 @@ class ManagedBufferPointerType extends BoundGenericType {
6565
private class PointerSummaries extends SummaryModelCsv {
6666
override predicate row(string row) {
6767
row =
68-
";UnsafeMutableBufferPointer;true;update(repeating:);;;Argument[0];Argument[-1].CollectionElement;value"
68+
[
69+
";UnsafeMutablePointer;true;init(mutating:);;;Argument[0];ReturnValue;taint",
70+
";UnsafeMutableBufferPointer;true;update(repeating:);;;Argument[0];Argument[-1].CollectionElement;value",
71+
]
6972
}
7073
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ private import NsData
1414
private import NsObject
1515
private import NsString
1616
private import NsUrl
17+
private import PointerTypes
1718
private import Sequence
1819
private import Set
1920
private import String

swift/ql/lib/codeql/swift/security/PathInjectionExtensions.qll

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,34 @@ private class DefaultPathInjectionSink extends PathInjectionSink {
3333
DefaultPathInjectionSink() { sinkNode(this, "path-injection") }
3434
}
3535

36+
/**
37+
* A sink that is a write to a global variable.
38+
*/
39+
private class GlobalVariablePathInjectionSink extends PathInjectionSink {
40+
GlobalVariablePathInjectionSink() {
41+
// value assigned to the sqlite3 global variable `sqlite3_temp_directory`
42+
// the sink should be the `DeclRefExpr` itself, but we don't currently have taint flow to globals.
43+
exists(AssignExpr ae |
44+
ae.getDest().(DeclRefExpr).getDecl().(VarDecl).getName() = "sqlite3_temp_directory" and
45+
ae.getSource() = this.asExpr()
46+
)
47+
}
48+
}
49+
50+
/**
51+
* A sink that is an argument to an enum element.
52+
*/
53+
private class EnumConstructorPathInjectionSink extends PathInjectionSink {
54+
EnumConstructorPathInjectionSink() {
55+
// first argument to SQLite.swift's `Connection.Location.uri(_:parameters:)`
56+
exists(ApplyExpr ae, EnumElementDecl decl |
57+
ae.getFunction().(MethodLookupExpr).getMember() = decl and
58+
decl.hasQualifiedName("Connection.Location", "uri") and
59+
this.asExpr() = ae.getArgument(0).getExpr()
60+
)
61+
}
62+
}
63+
3664
private class DefaultPathInjectionBarrier extends PathInjectionBarrier {
3765
DefaultPathInjectionBarrier() {
3866
// This is a simplified implementation.
@@ -130,6 +158,17 @@ private class PathInjectionSinks extends SinkModelCsv {
130158
";Realm.Configuration;true;init(fileURL:inMemoryIdentifier:syncConfiguration:encryptionKey:readOnly:schemaVersion:migrationBlock:deleteRealmIfMigrationNeeded:shouldCompactOnLaunch:objectTypes:seedFilePath:);;;Argument[10];path-injection",
131159
";Realm.Configuration;true;fileURL;;;PostUpdate;path-injection",
132160
";Realm.Configuration;true;seedFilePath;;;PostUpdate;path-injection",
161+
// sqlite3
162+
";;false;sqlite3_open(_:_:);;;Argument[0];path-injection",
163+
";;false;sqlite3_open16(_:_:);;;Argument[0];path-injection",
164+
";;false;sqlite3_open_v2(_:_:_:_:);;;Argument[0];path-injection",
165+
";;false;sqlite3_database_file_object(_:);;;Argument[0];path-injection",
166+
";;false;sqlite3_filename_database(_:);;;Argument[0];path-injection",
167+
";;false;sqlite3_filename_journal(_:);;;Argument[0];path-injection",
168+
";;false;sqlite3_filename_wal(_:);;;Argument[0];path-injection",
169+
";;false;sqlite3_free_filename(_:);;;Argument[0];path-injection",
170+
// SQLite.swift
171+
";Connection;true;init(_:readonly:);;;Argument[0];path-injection",
133172
]
134173
}
135174
}
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Added sqlite3 and SQLite.swift path injection sinks for the `swift/path-injection` query.

0 commit comments

Comments
 (0)