Merge pull request #8817 from atorralba/atorralba/cleartext-storage-sharedprefs-improvs

Java: Add value-preserving flow steps for Android's SharedPreferences

Author: atorralba

java/ql/lib/change-notes/2022-04-22-sharedprefs-fluent-steps.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+Improved the data flow support for the Android class `SharedPreferences$Editor`. Specifically, the fluent logic of some of its methods is now taken into account when calculating data flow.

java/ql/lib/semmle/code/java/dataflow/ExternalFlow.qll

@@ -82,6 +82,7 @@ private module Frameworks {
   private import semmle.code.java.frameworks.android.ContentProviders
   private import semmle.code.java.frameworks.android.Intent
   private import semmle.code.java.frameworks.android.Notifications
+  private import semmle.code.java.frameworks.android.SharedPreferences
   private import semmle.code.java.frameworks.android.Slice
   private import semmle.code.java.frameworks.android.SQLite
   private import semmle.code.java.frameworks.android.Widget

java/ql/lib/semmle/code/java/frameworks/android/SharedPreferences.qll

@@ -1,6 +1,7 @@
 /** Provides classes related to `android.content.SharedPreferences`. */
 
 import java
+private import semmle.code.java.dataflow.ExternalFlow
 
 /** The interface `android.content.SharedPreferences`. */
 class SharedPreferences extends Interface {
@@ -55,3 +56,19 @@ class StoreSharedPreferenceMethod extends Method {
     this.hasName(["commit", "apply"])
   }
 }
+
+private class SharedPreferencesSummaries extends SummaryModelCsv {
+  override predicate row(string row) {
+    row =
+      [
+        "android.content;SharedPreferences$Editor;true;clear;;;Argument[-1];ReturnValue;value",
+        "android.content;SharedPreferences$Editor;true;putBoolean;;;Argument[-1];ReturnValue;value",
+        "android.content;SharedPreferences$Editor;true;putFloat;;;Argument[-1];ReturnValue;value",
+        "android.content;SharedPreferences$Editor;true;putInt;;;Argument[-1];ReturnValue;value",
+        "android.content;SharedPreferences$Editor;true;putLong;;;Argument[-1];ReturnValue;value",
+        "android.content;SharedPreferences$Editor;true;putString;;;Argument[-1];ReturnValue;value",
+        "android.content;SharedPreferences$Editor;true;putStringSet;;;Argument[-1];ReturnValue;value",
+        "android.content;SharedPreferences$Editor;true;remove;;;Argument[-1];ReturnValue;value"
+      ]
+  }
+}

java/ql/test/query-tests/security/CWE-312/CleartextStorageSharedPrefsTest.java

@@ -89,9 +89,16 @@ public void testSetSharedPrefs6(Context context, String name, String password)
 				.create(context, "secret_shared_prefs", masterKey,
 						EncryptedSharedPreferences.PrefKeyEncryptionScheme.AES256_SIV,
 						EncryptedSharedPreferences.PrefValueEncryptionScheme.AES256_GCM)
-				.edit().putString("name", name) /// Safe
+				.edit().putString("name", name) // Safe
 				.putString("password", password); // Safe
 
 		editor.commit();
 	}
+
+	public void testSetSharedPrefs7(Context context, String name, String password) {
+		SharedPreferences sharedPrefs =
+				context.getSharedPreferences("user_prefs", Context.MODE_PRIVATE);
+		sharedPrefs.edit().putString("name", name).apply(); // Safe
+		sharedPrefs.edit().putString("password", password).apply(); // $hasCleartextStorageSharedPrefs
+	}
 }