Merge pull request #10689 from d10c/swift/cleartext-storage-nsuserdefaults

Swift: Query for CWE-312: Exposure of sensitive information using NSUserDefaults
2026-04-29 18:55:14 +02:00 · 2022-10-17 14:05:17 +01:00
parent 13f9834fde b7ad287cb1
commit 0281bfedda
9 changed files with 357 additions and 7 deletions
--- a/swift/README.md
+++ b/swift/README.md
@@ -1,8 +1,10 @@
+# Swift on CodeQL
+
 ## Warning

-The Swift codeql package is an experimental and unsupported work in progress.
+The Swift CodeQL package is an experimental and unsupported work in progress.

-## Usage
+## Building the Swift extractor

 First ensure you have Bazel installed, for example with

@@ -13,16 +15,17 @@ brew install bazelisk
 then from the `ql` directory run

 ```bash
-bazel run //swift:create-extractor-pack
+bazel run //swift:create-extractor-pack    # --cpu=darwin_x86_64 # Uncomment on Arm-based Macs
 ```

 which will install `swift/extractor-pack`.

-Using `--search-path=swift/extractor-pack` will then pick up the Swift extractor. You can also use
-`--search-path=.`, as the extractor pack is mentioned in the root `codeql-workspace.yml`.
-
 Notice you can run `bazel run :create-extractor-pack` if you already are in the `swift` directory.

+Using `codeql ... --search-path=swift/extractor-pack` will then pick up the Swift extractor. You can also use
+`--search-path=.`, as the extractor pack is mentioned in the root `codeql-workspace.yml`. Alternatively, you can
+set up the search path in [the per-user CodeQL configuration file](https://codeql.github.com/docs/codeql-cli/specifying-command-options-in-a-codeql-configuration-file/#using-a-codeql-configuration-file).
+
 ## Code generation

 Run
--- a/swift/ql/src/queries/Security/CWE-312/CleartextStoragePreferences.qhelp
+++ b/swift/ql/src/queries/Security/CWE-312/CleartextStoragePreferences.qhelp
@@ -0,0 +1,31 @@
+<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
+<qhelp>
+<overview>
+
+<p>Sensitive information that is stored unencrypted in an application preference store, such as the user defaults database or the iCloud-backed ubiquitous key-value store, is accessible to an attacker who gains access to that data store. For example, the information could be accessed by any process or user in a rooted device, by compromised app extensions, or could be exposed through another vulnerability.</p>
+
+</overview>
+<recommendation>
+
+<p>Either store the data in an encrypted database, or ensure that each piece of sensitive information is encrypted before being stored. In general, decrypt sensitive information only at the point where it is necessary for it to be used in cleartext. Avoid storing sensitive information at all if you do not need to keep it.</p>
+
+</recommendation>
+<example>
+
+<p>The following example shows three cases of storing information using UserDefaults. In the 'BAD' case, the data that is stored is sensitive (a credit card number) and is not encrypted.  In the 'GOOD' cases, the data is either not sensitive, or is protected with encryption.</p>
+
+<sample src="CleartextStoragePreferences.swift" />
+
+</example>
+<references>
+
+<li>
+  OWASP Top 10:2021:
+  <a href="https://owasp.org/Top10/A02_2021-Cryptographic_Failures/">A02:2021 &mdash; Cryptographic Failures</a>.
+</li>
+<li>
+Apple Developer Documentation: <a href="https://developer.apple.com/documentation/foundation/userdefaults">UserDefaults</a>, <a href="https://developer.apple.com/documentation/foundation/nsubiquitouskeyvaluestore">NSUbiquitousKeyValueStore</a>
+</li>
+
+</references>
+</qhelp>
--- a/swift/ql/src/queries/Security/CWE-312/CleartextStoragePreferences.ql
+++ b/swift/ql/src/queries/Security/CWE-312/CleartextStoragePreferences.ql
@@ -0,0 +1,106 @@
+/**
+ * @name Cleartext storage of sensitive information in an application preference store
+ * @description Storing sensitive information in a non-encrypted store can expose it to an attacker.
+ * @kind path-problem
+ * @problem.severity warning
+ * @security-severity 7.5
+ * @precision medium
+ * @id swift/cleartext-storage-preferences
+ * @tags security
+ *       external/cwe/cwe-312
+ */
+
+import swift
+import codeql.swift.security.SensitiveExprs
+import codeql.swift.dataflow.DataFlow
+import codeql.swift.dataflow.TaintTracking
+import DataFlow::PathGraph
+
+/**
+ * A `DataFlow::Node` of something that gets stored in an application preference store.
+ */
+abstract class Stored extends DataFlow::Node {
+  abstract string getStoreName();
+}
+
+/** The `DataFlow::Node` of an expression that gets written to the user defaults database */
+class UserDefaultsStore extends Stored {
+  UserDefaultsStore() {
+    exists(ClassDecl c, AbstractFunctionDecl f, CallExpr call |
+      c.getName() = "UserDefaults" and
+      c.getAMember() = f and
+      f.getName() = "set(_:forKey:)" and
+      call.getStaticTarget() = f and
+      call.getArgument(0).getExpr() = this.asExpr()
+    )
+  }
+
+  override string getStoreName() { result = "the user defaults database" }
+}
+
+/** The `DataFlow::Node` of an expression that gets written to the iCloud-backed NSUbiquitousKeyValueStore */
+class NSUbiquitousKeyValueStore extends Stored {
+  NSUbiquitousKeyValueStore() {
+    exists(ClassDecl c, AbstractFunctionDecl f, CallExpr call |
+      c.getName() = "NSUbiquitousKeyValueStore" and
+      c.getAMember() = f and
+      f.getName() = "set(_:forKey:)" and
+      call.getStaticTarget() = f and
+      call.getArgument(0).getExpr() = this.asExpr()
+    )
+  }
+
+  override string getStoreName() { result = "iCloud" }
+}
+
+/**
+ * A more complicated case, this is a macOS-only way of writing to
+ * NSUserDefaults by modifying the `NSUserDefaultsController.values: Any`
+ * object via reflection (`perform(Selector)`) or the `NSKeyValueCoding`,
+ * `NSKeyValueBindingCreation` APIs. (TODO)
+ */
+class NSUserDefaultsControllerStore extends Stored {
+  NSUserDefaultsControllerStore() { none() }
+
+  override string getStoreName() { result = "the user defaults database" }
+}
+
+/**
+ * A taint configuration from sensitive information to expressions that are
+ * stored as preferences.
+ */
+class CleartextStorageConfig extends TaintTracking::Configuration {
+  CleartextStorageConfig() { this = "CleartextStorageConfig" }
+
+  override predicate isSource(DataFlow::Node node) { node.asExpr() instanceof SensitiveExpr }
+
+  override predicate isSink(DataFlow::Node node) { node instanceof Stored }
+
+  override predicate isSanitizerIn(DataFlow::Node node) {
+    // make sources barriers so that we only report the closest instance
+    this.isSource(node)
+  }
+
+  override predicate isSanitizer(DataFlow::Node node) {
+    // encryption barrier
+    node.asExpr() instanceof EncryptedExpr
+  }
+}
+
+/**
+ * Gets a prettier node to use in the results.
+ */
+DataFlow::Node cleanupNode(DataFlow::Node n) {
+  result = n.(DataFlow::PostUpdateNode).getPreUpdateNode()
+  or
+  not n instanceof DataFlow::PostUpdateNode and
+  result = n
+}
+
+from CleartextStorageConfig config, DataFlow::PathNode sourceNode, DataFlow::PathNode sinkNode
+where config.hasFlowPath(sourceNode, sinkNode)
+select cleanupNode(sinkNode.getNode()), sourceNode, sinkNode,
+  "This operation stores '" + sinkNode.getNode().toString() + "' in " +
+    sinkNode.getNode().(Stored).getStoreName() +
+    ". It may contain unencrypted sensitive data from $@.", sourceNode,
+  sourceNode.getNode().toString()
--- a/swift/ql/src/queries/Security/CWE-312/CleartextStoragePreferences.swift
+++ b/swift/ql/src/queries/Security/CWE-312/CleartextStoragePreferences.swift
@@ -0,0 +1,15 @@
+
+func storeMyData(faveSong : String, creditCardNo : String) {
+	// ...
+
+	// GOOD: not sensitive information
+	UserDefaults.standard.set(faveSong, forKey: "myFaveSong")
+
+	// BAD: sensitive information saved in cleartext
+	UserDefaults.standard.set(creditCardNo, forKey: "myCreditCardNo")
+
+	// GOOD: encrypted sensitive information saved
+	UserDefaults.standard.set(encrypt(creditCardNo), forKey: "myCreditCardNo")
+
+	// ...
+}
--- a/swift/ql/test/query-tests/Security/CWE-312/CleartextStoragePreferences.expected
+++ b/swift/ql/test/query-tests/Security/CWE-312/CleartextStoragePreferences.expected
@@ -0,0 +1,52 @@
+edges
+| testNSUbiquitousKeyValueStore.swift:41:24:41:24 | x :  | testNSUbiquitousKeyValueStore.swift:42:40:42:40 | x |
+| testNSUbiquitousKeyValueStore.swift:44:10:44:22 | call to getPassword() :  | testNSUbiquitousKeyValueStore.swift:45:40:45:40 | y |
+| testNSUbiquitousKeyValueStore.swift:55:10:55:10 | passwd :  | testNSUbiquitousKeyValueStore.swift:59:40:59:40 | x |
+| testNSUbiquitousKeyValueStore.swift:56:10:56:10 | passwd :  | testNSUbiquitousKeyValueStore.swift:60:40:60:40 | y |
+| testNSUbiquitousKeyValueStore.swift:57:10:57:10 | passwd :  | testNSUbiquitousKeyValueStore.swift:61:40:61:40 | z |
+| testUserDefaults.swift:41:24:41:24 | x :  | testUserDefaults.swift:42:28:42:28 | x |
+| testUserDefaults.swift:44:10:44:22 | call to getPassword() :  | testUserDefaults.swift:45:28:45:28 | y |
+| testUserDefaults.swift:55:10:55:10 | passwd :  | testUserDefaults.swift:59:28:59:28 | x |
+| testUserDefaults.swift:56:10:56:10 | passwd :  | testUserDefaults.swift:60:28:60:28 | y |
+| testUserDefaults.swift:57:10:57:10 | passwd :  | testUserDefaults.swift:61:28:61:28 | z |
+nodes
+| testNSUbiquitousKeyValueStore.swift:28:12:28:12 | password | semmle.label | password |
+| testNSUbiquitousKeyValueStore.swift:41:24:41:24 | x :  | semmle.label | x :  |
+| testNSUbiquitousKeyValueStore.swift:42:40:42:40 | x | semmle.label | x |
+| testNSUbiquitousKeyValueStore.swift:44:10:44:22 | call to getPassword() :  | semmle.label | call to getPassword() :  |
+| testNSUbiquitousKeyValueStore.swift:45:40:45:40 | y | semmle.label | y |
+| testNSUbiquitousKeyValueStore.swift:49:40:49:42 | .password | semmle.label | .password |
+| testNSUbiquitousKeyValueStore.swift:55:10:55:10 | passwd :  | semmle.label | passwd :  |
+| testNSUbiquitousKeyValueStore.swift:56:10:56:10 | passwd :  | semmle.label | passwd :  |
+| testNSUbiquitousKeyValueStore.swift:57:10:57:10 | passwd :  | semmle.label | passwd :  |
+| testNSUbiquitousKeyValueStore.swift:59:40:59:40 | x | semmle.label | x |
+| testNSUbiquitousKeyValueStore.swift:60:40:60:40 | y | semmle.label | y |
+| testNSUbiquitousKeyValueStore.swift:61:40:61:40 | z | semmle.label | z |
+| testUserDefaults.swift:28:15:28:15 | password | semmle.label | password |
+| testUserDefaults.swift:41:24:41:24 | x :  | semmle.label | x :  |
+| testUserDefaults.swift:42:28:42:28 | x | semmle.label | x |
+| testUserDefaults.swift:44:10:44:22 | call to getPassword() :  | semmle.label | call to getPassword() :  |
+| testUserDefaults.swift:45:28:45:28 | y | semmle.label | y |
+| testUserDefaults.swift:49:28:49:30 | .password | semmle.label | .password |
+| testUserDefaults.swift:55:10:55:10 | passwd :  | semmle.label | passwd :  |
+| testUserDefaults.swift:56:10:56:10 | passwd :  | semmle.label | passwd :  |
+| testUserDefaults.swift:57:10:57:10 | passwd :  | semmle.label | passwd :  |
+| testUserDefaults.swift:59:28:59:28 | x | semmle.label | x |
+| testUserDefaults.swift:60:28:60:28 | y | semmle.label | y |
+| testUserDefaults.swift:61:28:61:28 | z | semmle.label | z |
+subpaths
+#select
+| testNSUbiquitousKeyValueStore.swift:28:12:28:12 | password | testNSUbiquitousKeyValueStore.swift:28:12:28:12 | password | testNSUbiquitousKeyValueStore.swift:28:12:28:12 | password | This operation stores 'password' in iCloud. It may contain unencrypted sensitive data from $@. | testNSUbiquitousKeyValueStore.swift:28:12:28:12 | password | password |
+| testNSUbiquitousKeyValueStore.swift:42:40:42:40 | x | testNSUbiquitousKeyValueStore.swift:41:24:41:24 | x :  | testNSUbiquitousKeyValueStore.swift:42:40:42:40 | x | This operation stores 'x' in iCloud. It may contain unencrypted sensitive data from $@. | testNSUbiquitousKeyValueStore.swift:41:24:41:24 | x :  | x |
+| testNSUbiquitousKeyValueStore.swift:45:40:45:40 | y | testNSUbiquitousKeyValueStore.swift:44:10:44:22 | call to getPassword() :  | testNSUbiquitousKeyValueStore.swift:45:40:45:40 | y | This operation stores 'y' in iCloud. It may contain unencrypted sensitive data from $@. | testNSUbiquitousKeyValueStore.swift:44:10:44:22 | call to getPassword() :  | call to getPassword() |
+| testNSUbiquitousKeyValueStore.swift:49:40:49:42 | .password | testNSUbiquitousKeyValueStore.swift:49:40:49:42 | .password | testNSUbiquitousKeyValueStore.swift:49:40:49:42 | .password | This operation stores '.password' in iCloud. It may contain unencrypted sensitive data from $@. | testNSUbiquitousKeyValueStore.swift:49:40:49:42 | .password | .password |
+| testNSUbiquitousKeyValueStore.swift:59:40:59:40 | x | testNSUbiquitousKeyValueStore.swift:55:10:55:10 | passwd :  | testNSUbiquitousKeyValueStore.swift:59:40:59:40 | x | This operation stores 'x' in iCloud. It may contain unencrypted sensitive data from $@. | testNSUbiquitousKeyValueStore.swift:55:10:55:10 | passwd :  | passwd |
+| testNSUbiquitousKeyValueStore.swift:60:40:60:40 | y | testNSUbiquitousKeyValueStore.swift:56:10:56:10 | passwd :  | testNSUbiquitousKeyValueStore.swift:60:40:60:40 | y | This operation stores 'y' in iCloud. It may contain unencrypted sensitive data from $@. | testNSUbiquitousKeyValueStore.swift:56:10:56:10 | passwd :  | passwd |
+| testNSUbiquitousKeyValueStore.swift:61:40:61:40 | z | testNSUbiquitousKeyValueStore.swift:57:10:57:10 | passwd :  | testNSUbiquitousKeyValueStore.swift:61:40:61:40 | z | This operation stores 'z' in iCloud. It may contain unencrypted sensitive data from $@. | testNSUbiquitousKeyValueStore.swift:57:10:57:10 | passwd :  | passwd |
+| testUserDefaults.swift:28:15:28:15 | password | testUserDefaults.swift:28:15:28:15 | password | testUserDefaults.swift:28:15:28:15 | password | This operation stores 'password' in the user defaults database. It may contain unencrypted sensitive data from $@. | testUserDefaults.swift:28:15:28:15 | password | password |
+| testUserDefaults.swift:42:28:42:28 | x | testUserDefaults.swift:41:24:41:24 | x :  | testUserDefaults.swift:42:28:42:28 | x | This operation stores 'x' in the user defaults database. It may contain unencrypted sensitive data from $@. | testUserDefaults.swift:41:24:41:24 | x :  | x |
+| testUserDefaults.swift:45:28:45:28 | y | testUserDefaults.swift:44:10:44:22 | call to getPassword() :  | testUserDefaults.swift:45:28:45:28 | y | This operation stores 'y' in the user defaults database. It may contain unencrypted sensitive data from $@. | testUserDefaults.swift:44:10:44:22 | call to getPassword() :  | call to getPassword() |
+| testUserDefaults.swift:49:28:49:30 | .password | testUserDefaults.swift:49:28:49:30 | .password | testUserDefaults.swift:49:28:49:30 | .password | This operation stores '.password' in the user defaults database. It may contain unencrypted sensitive data from $@. | testUserDefaults.swift:49:28:49:30 | .password | .password |
+| testUserDefaults.swift:59:28:59:28 | x | testUserDefaults.swift:55:10:55:10 | passwd :  | testUserDefaults.swift:59:28:59:28 | x | This operation stores 'x' in the user defaults database. It may contain unencrypted sensitive data from $@. | testUserDefaults.swift:55:10:55:10 | passwd :  | passwd |
+| testUserDefaults.swift:60:28:60:28 | y | testUserDefaults.swift:56:10:56:10 | passwd :  | testUserDefaults.swift:60:28:60:28 | y | This operation stores 'y' in the user defaults database. It may contain unencrypted sensitive data from $@. | testUserDefaults.swift:56:10:56:10 | passwd :  | passwd |
+| testUserDefaults.swift:61:28:61:28 | z | testUserDefaults.swift:57:10:57:10 | passwd :  | testUserDefaults.swift:61:28:61:28 | z | This operation stores 'z' in the user defaults database. It may contain unencrypted sensitive data from $@. | testUserDefaults.swift:57:10:57:10 | passwd :  | passwd |
--- a/swift/ql/test/query-tests/Security/CWE-312/CleartextStoragePreferences.qlref
+++ b/swift/ql/test/query-tests/Security/CWE-312/CleartextStoragePreferences.qlref
@@ -0,0 +1 @@
+queries/Security/CWE-312/CleartextStoragePreferences.ql
--- a/swift/ql/test/query-tests/Security/CWE-312/testNSUbiquitousKeyValueStore.swift
+++ b/swift/ql/test/query-tests/Security/CWE-312/testNSUbiquitousKeyValueStore.swift
@@ -0,0 +1,70 @@
+
+// --- stubs ---
+
+class NSObject
+{
+}
+
+class NSUbiquitousKeyValueStore : NSObject
+{
+    class var `default`: NSUbiquitousKeyValueStore {
+        return NSUbiquitousKeyValueStore()
+    }
+
+	func set(_ anObject: Any?, forKey aKey: String) {}
+}
+
+func encrypt(_ data: String) -> String { return data }
+func hash(data: inout String) { }
+
+func getPassword() -> String { return "" }
+func doSomething(password: String) { }
+
+// --- tests ---
+
+func test1(password: String, passwordHash : String) {
+	let store = NSUbiquitousKeyValueStore.default
+
+	store.set(password, forKey: "myKey") // BAD
+	store.set(passwordHash, forKey: "myKey") // GOOD (not sensitive)
+}
+
+class MyClass {
+	var harmless = "abc"
+	var password = "123"
+}
+
+func test3(x: String) {
+	// alternative evidence of sensitivity...
+
+	NSUbiquitousKeyValueStore.default.set(x, forKey: "myKey") // BAD [NOT REPORTED]
+	doSomething(password: x);
+	NSUbiquitousKeyValueStore.default.set(x, forKey: "myKey") // BAD
+
+	let y = getPassword();
+	NSUbiquitousKeyValueStore.default.set(y, forKey: "myKey") // BAD
+
+	let z = MyClass()
+	NSUbiquitousKeyValueStore.default.set(z.harmless, forKey: "myKey") // GOOD (not sensitive)
+	NSUbiquitousKeyValueStore.default.set(z.password, forKey: "myKey") // BAD
+}
+
+func test4(passwd: String) {
+	// sanitizers...
+
+	var x = passwd;
+	var y = passwd;
+	var z = passwd;
+
+	NSUbiquitousKeyValueStore.default.set(x, forKey: "myKey") // BAD
+	NSUbiquitousKeyValueStore.default.set(y, forKey: "myKey") // BAD
+	NSUbiquitousKeyValueStore.default.set(z, forKey: "myKey") // BAD
+
+	x = encrypt(x);
+	hash(data: &y);
+	z = "";
+
+	NSUbiquitousKeyValueStore.default.set(x, forKey: "myKey") // GOOD (not sensitive)
+	NSUbiquitousKeyValueStore.default.set(y, forKey: "myKey") // GOOD (not sensitive)
+	NSUbiquitousKeyValueStore.default.set(z, forKey: "myKey") // GOOD (not sensitive)
+}
--- a/swift/ql/test/query-tests/Security/CWE-312/testUserDefaults.swift
+++ b/swift/ql/test/query-tests/Security/CWE-312/testUserDefaults.swift
@@ -0,0 +1,70 @@
+
+// --- stubs ---
+
+class NSObject
+{
+}
+
+class UserDefaults : NSObject
+{
+    class var standard: UserDefaults {
+        return UserDefaults()
+    }
+
+	func set(_ value: Any?, forKey defaultName: String) {}
+}
+
+func encrypt(_ data: String) -> String { return data }
+func hash(data: inout String) { }
+
+func getPassword() -> String { return "" }
+func doSomething(password: String) { }
+
+// --- tests ---
+
+func test1(password: String, passwordHash : String) {
+	let defaults = UserDefaults.standard
+
+	defaults.set(password, forKey: "myKey") // BAD
+	defaults.set(passwordHash, forKey: "myKey") // GOOD (not sensitive)
+}
+
+class MyClass {
+	var harmless = "abc"
+	var password = "123"
+}
+
+func test3(x: String) {
+	// alternative evidence of sensitivity...
+
+	UserDefaults.standard.set(x, forKey: "myKey") // BAD [NOT REPORTED]
+	doSomething(password: x);
+	UserDefaults.standard.set(x, forKey: "myKey") // BAD
+
+	let y = getPassword();
+	UserDefaults.standard.set(y, forKey: "myKey") // BAD
+
+	let z = MyClass()
+	UserDefaults.standard.set(z.harmless, forKey: "myKey") // GOOD (not sensitive)
+	UserDefaults.standard.set(z.password, forKey: "myKey") // BAD
+}
+
+func test4(passwd: String) {
+	// sanitizers...
+
+	var x = passwd;
+	var y = passwd;
+	var z = passwd;
+
+	UserDefaults.standard.set(x, forKey: "myKey") // BAD
+	UserDefaults.standard.set(y, forKey: "myKey") // BAD
+	UserDefaults.standard.set(z, forKey: "myKey") // BAD
+
+	x = encrypt(x);
+	hash(data: &y);
+	z = "";
+
+	UserDefaults.standard.set(x, forKey: "myKey") // GOOD (not sensitive)
+	UserDefaults.standard.set(y, forKey: "myKey") // GOOD (not sensitive)
+	UserDefaults.standard.set(z, forKey: "myKey") // GOOD (not sensitive)
+}
--- a/swift/tools/qltest.sh
+++ b/swift/tools/qltest.sh
@@ -10,5 +10,7 @@ for src in *.swift; do
  opts=(-sdk "$CODEQL_EXTRACTOR_SWIFT_ROOT/qltest/$CODEQL_PLATFORM/sdk" -c -primary-file $src)
  opts+=($(sed -n '1 s=//codeql-extractor-options:==p' $src))
  echo -e "calling extractor with flags: ${opts[@]}\n" >> $QLTEST_LOG
-  "$CODEQL_EXTRACTOR_SWIFT_ROOT/tools/$CODEQL_PLATFORM/extractor" "${opts[@]}" >> $QLTEST_LOG 2>&1
+  "$CODEQL_EXTRACTOR_SWIFT_ROOT/tools/$CODEQL_PLATFORM/extractor" "${opts[@]}" >> $QLTEST_LOG 2>&1 || FAILED=1
 done
+
+[ -z "$FAILED" ] || cat "$QLTEST_LOG" # Show compiler errors on extraction failure
				`@@ -0,0 +1 @@`
				`queries/Security/CWE-312/CleartextStoragePreferences.ql`