diff --git a/java/ql/lib/semmle/code/java/security/CleartextStorageSharedPrefsQuery.qll b/java/ql/lib/semmle/code/java/security/CleartextStorageSharedPrefsQuery.qll new file mode 100644 index 000000000000..30a8ffc3a120 --- /dev/null +++ b/java/ql/lib/semmle/code/java/security/CleartextStorageSharedPrefsQuery.qll @@ -0,0 +1,79 @@ +/** Provides classes and predicates to reason about cleartext storage in Android's SharedPreferences. */ + +import java +import semmle.code.java.dataflow.DataFlow +import semmle.code.java.frameworks.android.SharedPreferences +import semmle.code.java.security.CleartextStorageQuery + +private class SharedPrefsCleartextStorageSink extends CleartextStorageSink { + SharedPrefsCleartextStorageSink() { + exists(MethodAccess m | + m.getMethod() instanceof PutSharedPreferenceMethod and + this.asExpr() = m.getArgument(1) + ) + } +} + +/** + * The call to get a `SharedPreferences.Editor` object, which can set shared preferences and be + * stored to the device. + */ +class SharedPreferencesEditorMethodAccess extends Storable, MethodAccess { + SharedPreferencesEditorMethodAccess() { + this.getMethod() instanceof GetSharedPreferencesEditorMethod and + not DataFlow::localExprFlow(any(MethodAccess ma | + ma.getMethod() instanceof CreateEncryptedSharedPreferencesMethod + ), this.getQualifier()) + } + + /** Gets an input, for example `password` in `editor.putString("password", password);`. */ + override Expr getAnInput() { + exists(SharedPreferencesFlowConfig conf, DataFlow::Node editor | + sharedPreferencesInput(editor, result) and + conf.hasFlow(DataFlow::exprNode(this), editor) + ) + } + + /** Gets a store, for example `editor.commit();`. */ + override Expr getAStore() { + exists(SharedPreferencesFlowConfig conf, DataFlow::Node editor | + sharedPreferencesStore(editor, result) and + conf.hasFlow(DataFlow::exprNode(this), editor) + ) + } +} + +/** + * Holds if `input` is the second argument of a setter method + * called on `editor`, which is an instance of `SharedPreferences$Editor`. + */ +private predicate sharedPreferencesInput(DataFlow::Node editor, Expr input) { + exists(MethodAccess m | + m.getMethod() instanceof PutSharedPreferenceMethod and + input = m.getArgument(1) and + editor.asExpr() = m.getQualifier() + ) +} + +/** + * Holds if `m` is a store method called on `editor`, + * which is an instance of `SharedPreferences$Editor`. + */ +private predicate sharedPreferencesStore(DataFlow::Node editor, MethodAccess m) { + m.getMethod() instanceof StoreSharedPreferenceMethod and + editor.asExpr() = m.getQualifier() +} + +/** Flow from `SharedPreferences.Editor` to either a setter or a store method. */ +private class SharedPreferencesFlowConfig extends DataFlow::Configuration { + SharedPreferencesFlowConfig() { this = "SharedPreferencesFlowConfig" } + + override predicate isSource(DataFlow::Node src) { + src.asExpr() instanceof SharedPreferencesEditorMethodAccess + } + + override predicate isSink(DataFlow::Node sink) { + sharedPreferencesInput(sink, _) or + sharedPreferencesStore(sink, _) + } +} diff --git a/java/ql/src/experimental/Security/CWE/CWE-312/CleartextStorageSharedPrefs.java b/java/ql/src/Security/CWE/CWE-312/CleartextStorageSharedPrefs.java similarity index 65% rename from java/ql/src/experimental/Security/CWE/CWE-312/CleartextStorageSharedPrefs.java rename to java/ql/src/Security/CWE/CWE-312/CleartextStorageSharedPrefs.java index f4fbeaa230b4..49afdffca926 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-312/CleartextStorageSharedPrefs.java +++ b/java/ql/src/Security/CWE/CWE-312/CleartextStorageSharedPrefs.java @@ -1,7 +1,7 @@ public void testSetSharedPrefs(Context context, String name, String password) { { - // BAD - save sensitive information in cleartext + // BAD - sensitive information saved in cleartext. SharedPreferences sharedPrefs = context.getSharedPreferences("user_prefs", Context.MODE_PRIVATE); Editor editor = sharedPrefs.edit(); editor.putString("name", name); @@ -10,7 +10,7 @@ public void testSetSharedPrefs(Context context, String name, String password) } { - // GOOD - save sensitive information in encrypted format + // GOOD - save sensitive information encrypted with a custom method. SharedPreferences sharedPrefs = context.getSharedPreferences("user_prefs", Context.MODE_PRIVATE); Editor editor = sharedPrefs.edit(); editor.putString("name", encrypt(name)); @@ -19,7 +19,7 @@ public void testSetSharedPrefs(Context context, String name, String password) } { - // GOOD - save sensitive information using the built-in `EncryptedSharedPreferences` class in androidx. + // GOOD - sensitive information saved using the built-in `EncryptedSharedPreferences` class in androidx. MasterKey masterKey = new MasterKey.Builder(context, MasterKey.DEFAULT_MASTER_KEY_ALIAS) .setKeyScheme(MasterKey.KeyScheme.AES256_GCM) .build(); @@ -37,3 +37,12 @@ public void testSetSharedPrefs(Context context, String name, String password) editor.commit(); } } + +private static String encrypt(String cleartext) throws Exception { + // Use an encryption or hashing algorithm in real world. The demo below just returns its + // hash. + MessageDigest digest = MessageDigest.getInstance("SHA-256"); + byte[] hash = digest.digest(cleartext.getBytes(StandardCharsets.UTF_8)); + String encoded = Base64.getEncoder().encodeToString(hash); + return encoded; +} diff --git a/java/ql/src/experimental/Security/CWE/CWE-312/CleartextStorageSharedPrefs.qhelp b/java/ql/src/Security/CWE/CWE-312/CleartextStorageSharedPrefs.qhelp similarity index 62% rename from java/ql/src/experimental/Security/CWE/CWE-312/CleartextStorageSharedPrefs.qhelp rename to java/ql/src/Security/CWE/CWE-312/CleartextStorageSharedPrefs.qhelp index 90f87f30b91d..a046202af4f6 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-312/CleartextStorageSharedPrefs.qhelp +++ b/java/ql/src/Security/CWE/CWE-312/CleartextStorageSharedPrefs.qhelp @@ -2,7 +2,7 @@

- SharedPreferences is an Android API that stores application preferences using simple sets of data values. Almost every Android application uses this API. It allows to easily save, alter, and retrieve the values stored in the user's profile. However, sensitive information should not be saved in cleartext. Otherwise it can be accessed by any process or user on rooted devices, or can be disclosed through chained vulnerabilities e.g. unexpected access to its private storage through exposed components. + SharedPreferences is an Android API that stores application preferences using simple sets of data values. It allows you to easily save, alter, and retrieve the values stored in a user's profile. However, sensitive information should not be saved in cleartext. Otherwise it can be accessed by any process or user in rooted devices, or can be disclosed through chained vulnerabilities, like unexpected access to the private storage through exposed components.

@@ -24,10 +24,6 @@ -
  • - CWE: - CWE-312: Cleartext Storage of Sensitive Information -
  • Android Developers: Work with data more securely diff --git a/java/ql/src/Security/CWE/CWE-312/CleartextStorageSharedPrefs.ql b/java/ql/src/Security/CWE/CWE-312/CleartextStorageSharedPrefs.ql new file mode 100644 index 000000000000..60c5522e55b6 --- /dev/null +++ b/java/ql/src/Security/CWE/CWE-312/CleartextStorageSharedPrefs.ql @@ -0,0 +1,23 @@ +/** + * @name Cleartext storage of sensitive information using `SharedPreferences` on Android + * @description Cleartext Storage of Sensitive Information using + * SharedPreferences on Android allows access for users with root + * privileges or unexpected exposure from chained vulnerabilities. + * @kind problem + * @problem.severity warning + * @precision medium + * @id java/android/cleartext-storage-shared-prefs + * @tags security + * external/cwe/cwe-312 + */ + +import java +import semmle.code.java.security.CleartextStorageSharedPrefsQuery + +from SensitiveSource data, SharedPreferencesEditorMethodAccess s, Expr input, Expr store +where + input = s.getAnInput() and + store = s.getAStore() and + data.flowsTo(input) +select store, "'SharedPreferences' class $@ containing $@ is stored $@. Data was added $@.", s, + s.toString(), data, "sensitive data", store, "here", input, "here" diff --git a/java/ql/src/change-notes/2021-08-10-cleartext-storage-sharedprefs-query.md b/java/ql/src/change-notes/2021-08-10-cleartext-storage-sharedprefs-query.md new file mode 100644 index 000000000000..472b083e7e11 --- /dev/null +++ b/java/ql/src/change-notes/2021-08-10-cleartext-storage-sharedprefs-query.md @@ -0,0 +1,4 @@ +--- +category: newQuery +--- +* The query "Cleartext storage of sensitive information using `SharedPreferences` on Android" (`java/android/cleartext-storage-shared-prefs`) has been promoted from experimental to the main query pack. Its results will now appear by default. This query was originally [submitted as an experimental query by @luchua-bc](https://github.com/github/codeql/pull/4675). diff --git a/java/ql/src/experimental/Security/CWE/CWE-312/CleartextStorageSharedPrefs.ql b/java/ql/src/experimental/Security/CWE/CWE-312/CleartextStorageSharedPrefs.ql deleted file mode 100644 index b10741c20489..000000000000 --- a/java/ql/src/experimental/Security/CWE/CWE-312/CleartextStorageSharedPrefs.ql +++ /dev/null @@ -1,167 +0,0 @@ -/** - * @name Cleartext storage of sensitive information using `SharedPreferences` on Android - * @description Cleartext Storage of Sensitive Information using - * SharedPreferences on Android allows access for users with root - * privileges or unexpected exposure from chained vulnerabilities. - * @kind problem - * @problem.severity warning - * @precision medium - * @id java/android/cleartext-storage-shared-prefs - * @tags security - * external/cwe/cwe-312 - */ - -import java -import semmle.code.java.dataflow.DataFlow4 -import semmle.code.java.dataflow.DataFlow5 -import semmle.code.java.dataflow.TaintTracking -import semmle.code.java.frameworks.android.Intent -import semmle.code.java.frameworks.android.SharedPreferences -import semmle.code.java.security.SensitiveActions - -/** Holds if the method call is a setter method of `SharedPreferences`. */ -private predicate sharedPreferencesInput(DataFlow::Node sharedPrefs, Expr input) { - exists(MethodAccess m | - m.getMethod() instanceof PutSharedPreferenceMethod and - input = m.getArgument(1) and - not exists(EncryptedValueFlowConfig conf | conf.hasFlow(_, DataFlow::exprNode(input))) and - sharedPrefs.asExpr() = m.getQualifier() - ) -} - -/** Holds if the method call is the store method of `SharedPreferences`. */ -private predicate sharedPreferencesStore(DataFlow::Node sharedPrefs, Expr store) { - exists(MethodAccess m | - m.getMethod() instanceof StoreSharedPreferenceMethod and - store = m and - sharedPrefs.asExpr() = m.getQualifier() - ) -} - -/** Flow from `SharedPreferences` to either a setter or a store method. */ -class SharedPreferencesFlowConfig extends DataFlow::Configuration { - SharedPreferencesFlowConfig() { - this = "CleartextStorageSharedPrefs::SharedPreferencesFlowConfig" - } - - override predicate isSource(DataFlow::Node src) { - src.asExpr() instanceof SharedPreferencesEditorMethodAccess - } - - override predicate isSink(DataFlow::Node sink) { - sharedPreferencesInput(sink, _) or - sharedPreferencesStore(sink, _) - } -} - -/** - * Method call of encrypting sensitive information. - * As there are various implementations of encryption (reversible and non-reversible) from both JDK and third parties, this class simply checks method name to take a best guess to reduce false positives. - */ -class EncryptedSensitiveMethodAccess extends MethodAccess { - EncryptedSensitiveMethodAccess() { - this.getMethod().getName().toLowerCase().matches(["%encrypt%", "%hash%"]) - } -} - -/** Flow configuration of encrypting sensitive information. */ -class EncryptedValueFlowConfig extends DataFlow5::Configuration { - EncryptedValueFlowConfig() { this = "CleartextStorageSharedPrefs::EncryptedValueFlowConfig" } - - override predicate isSource(DataFlow5::Node src) { - exists(EncryptedSensitiveMethodAccess ema | src.asExpr() = ema) - } - - override predicate isSink(DataFlow5::Node sink) { - exists(MethodAccess ma | - ma.getMethod() instanceof PutSharedPreferenceMethod and - sink.asExpr() = ma.getArgument(1) - ) - } -} - -/** Flow from the create method of `androidx.security.crypto.EncryptedSharedPreferences` to its instance. */ -private class EncryptedSharedPrefFlowConfig extends DataFlow4::Configuration { - EncryptedSharedPrefFlowConfig() { - this = "CleartextStorageSharedPrefs::EncryptedSharedPrefFlowConfig" - } - - override predicate isSource(DataFlow4::Node src) { - src.asExpr().(MethodAccess).getMethod() instanceof CreateEncryptedSharedPreferencesMethod - } - - override predicate isSink(DataFlow4::Node sink) { - sink.asExpr().getType() instanceof SharedPreferences - } -} - -/** The call to get a `SharedPreferences.Editor` object, which can set shared preferences or be stored to device. */ -class SharedPreferencesEditorMethodAccess extends MethodAccess { - SharedPreferencesEditorMethodAccess() { - this.getMethod() instanceof GetSharedPreferencesEditorMethod and - not exists( - EncryptedSharedPrefFlowConfig config // not exists `SharedPreferences sharedPreferences = EncryptedSharedPreferences.create(...)` - | - config.hasFlow(_, DataFlow::exprNode(this.getQualifier())) - ) - } - - /** Gets an input, for example `password` in `editor.putString("password", password);`. */ - Expr getAnInput() { - exists(SharedPreferencesFlowConfig conf, DataFlow::Node n | - sharedPreferencesInput(n, result) and - conf.hasFlow(DataFlow::exprNode(this), n) - ) - } - - /** Gets a store, for example `editor.commit();`. */ - Expr getAStore() { - exists(SharedPreferencesFlowConfig conf, DataFlow::Node n | - sharedPreferencesStore(n, result) and - conf.hasFlow(DataFlow::exprNode(this), n) - ) - } -} - -/** - * Flow from sensitive expressions to shared preferences. - * Note it can be merged into `SensitiveSourceFlowConfig` of `Security/CWE/CWE-312/SensitiveStorage.qll` when this query is promoted from the experimental directory. - */ -private class SensitiveSharedPrefsFlowConfig extends TaintTracking::Configuration { - SensitiveSharedPrefsFlowConfig() { - this = "CleartextStorageSharedPrefs::SensitiveSharedPrefsFlowConfig" - } - - override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof SensitiveExpr } - - override predicate isSink(DataFlow::Node sink) { - exists(MethodAccess m | - m.getMethod() instanceof PutSharedPreferenceMethod and - sink.asExpr() = m.getArgument(1) - ) - } -} - -/** Class for shared preferences that may contain 'sensitive' information */ -class SensitiveSharedPrefsSource extends Expr { - SensitiveSharedPrefsSource() { - // SensitiveExpr is abstract, this lets us inherit from it without - // being a technical subclass - this instanceof SensitiveExpr - } - - /** Holds if this source flows to the `sink`. */ - predicate flowsTo(Expr sink) { - exists(SensitiveSharedPrefsFlowConfig conf | - conf.hasFlow(DataFlow::exprNode(this), DataFlow::exprNode(sink)) - ) - } -} - -from SensitiveSharedPrefsSource data, SharedPreferencesEditorMethodAccess s, Expr input, Expr store -where - input = s.getAnInput() and - store = s.getAStore() and - data.flowsTo(input) -select store, "'SharedPreferences' class $@ containing $@ is stored here. Data was added $@.", s, - s.toString(), data, "sensitive data", input, "here" diff --git a/java/ql/test/experimental/query-tests/security/CWE-312/CleartextStorageSharedPrefs.expected b/java/ql/test/experimental/query-tests/security/CWE-312/CleartextStorageSharedPrefs.expected deleted file mode 100644 index ab1db3fe099d..000000000000 --- a/java/ql/test/experimental/query-tests/security/CWE-312/CleartextStorageSharedPrefs.expected +++ /dev/null @@ -1 +0,0 @@ -| CleartextStorageSharedPrefs.java:19:3:19:17 | commit(...) | 'SharedPreferences' class $@ containing $@ is stored here. Data was added $@. | CleartextStorageSharedPrefs.java:16:19:16:36 | edit(...) | edit(...) | CleartextStorageSharedPrefs.java:18:32:18:39 | password | sensitive data | CleartextStorageSharedPrefs.java:18:32:18:39 | password | here | diff --git a/java/ql/test/experimental/query-tests/security/CWE-312/CleartextStorageSharedPrefs.java b/java/ql/test/experimental/query-tests/security/CWE-312/CleartextStorageSharedPrefs.java deleted file mode 100644 index 39614c82234b..000000000000 --- a/java/ql/test/experimental/query-tests/security/CWE-312/CleartextStorageSharedPrefs.java +++ /dev/null @@ -1,109 +0,0 @@ -import android.app.Activity; -import android.content.Context; -import android.content.SharedPreferences; -import android.content.SharedPreferences.Editor; -import androidx.security.crypto.MasterKey; -import androidx.security.crypto.EncryptedSharedPreferences; -import java.nio.charset.StandardCharsets; -import java.util.Base64; -import java.security.MessageDigest; - -/* Android activity that tests saving sensitive information in `SharedPreferences` */ -public class CleartextStorageSharedPrefs extends Activity { - // BAD - save sensitive information in cleartext - public void testSetSharedPrefs1(Context context, String name, String password) { - SharedPreferences sharedPrefs = context.getSharedPreferences("user_prefs", Context.MODE_PRIVATE); - Editor editor = sharedPrefs.edit(); - editor.putString("name", name); - editor.putString("password", password); - editor.commit(); - } - - // GOOD - save sensitive information in encrypted format - public void testSetSharedPrefs2(Context context, String name, String password) throws Exception { - SharedPreferences sharedPrefs = context.getSharedPreferences("user_prefs", Context.MODE_PRIVATE); - Editor editor = sharedPrefs.edit(); - editor.putString("name", encrypt(name)); - editor.putString("password", encrypt(password)); - editor.commit(); - } - - private static String encrypt(String cleartext) throws Exception { - // Use an encryption or hashing algorithm in real world. The demo below just returns its hash. - MessageDigest digest = MessageDigest.getInstance("SHA-256"); - byte[] hash = digest.digest(cleartext.getBytes(StandardCharsets.UTF_8)); - String encoded = Base64.getEncoder().encodeToString(hash); - return encoded; - } - - // GOOD - save sensitive information in encrypted format using separate variables - public void testSetSharedPrefs3(Context context, String name, String password) throws Exception { - String encUsername = encrypt(name); - String encPassword = encrypt(password); - SharedPreferences sharedPrefs = context.getSharedPreferences("user_prefs", Context.MODE_PRIVATE); - Editor editor = sharedPrefs.edit(); - editor.putString("name", encUsername); - editor.putString("password", encPassword); - editor.commit(); - } - - - // GOOD - save sensitive information using the built-in `EncryptedSharedPreferences` class in androidx - public void testSetSharedPrefs4(Context context, String name, String password) throws Exception { - MasterKey masterKey = new MasterKey.Builder(context, MasterKey.DEFAULT_MASTER_KEY_ALIAS) - .setKeyScheme(MasterKey.KeyScheme.AES256_GCM) - .build(); - - SharedPreferences sharedPreferences = EncryptedSharedPreferences.create( - context, - "secret_shared_prefs", - masterKey, - EncryptedSharedPreferences.PrefKeyEncryptionScheme.AES256_SIV, - EncryptedSharedPreferences.PrefValueEncryptionScheme.AES256_GCM); - - // Use the shared preferences and editor as you normally would - SharedPreferences.Editor editor = sharedPreferences.edit(); - editor.putString("name", name); - editor.putString("password", password); - editor.commit(); - } - - // GOOD - save sensitive information using the built-in `EncryptedSharedPreferences` class in androidx - public void testSetSharedPrefs5(Context context, String name, String password) throws Exception { - MasterKey masterKey = new MasterKey.Builder(context, MasterKey.DEFAULT_MASTER_KEY_ALIAS) - .setKeyScheme(MasterKey.KeyScheme.AES256_GCM) - .build(); - - SharedPreferences.Editor editor = EncryptedSharedPreferences.create( - context, - "secret_shared_prefs", - masterKey, - EncryptedSharedPreferences.PrefKeyEncryptionScheme.AES256_SIV, - EncryptedSharedPreferences.PrefValueEncryptionScheme.AES256_GCM) - .edit(); - - // Use the shared preferences and editor as you normally would - editor.putString("name", name); - editor.putString("password", password); - editor.commit(); - } - - // GOOD - save sensitive information using the built-in `EncryptedSharedPreferences` class in androidx - public void testSetSharedPrefs6(Context context, String name, String password) throws Exception { - MasterKey masterKey = new MasterKey.Builder(context, MasterKey.DEFAULT_MASTER_KEY_ALIAS) - .setKeyScheme(MasterKey.KeyScheme.AES256_GCM) - .build(); - - SharedPreferences.Editor editor = EncryptedSharedPreferences.create( - context, - "secret_shared_prefs", - masterKey, - EncryptedSharedPreferences.PrefKeyEncryptionScheme.AES256_SIV, - EncryptedSharedPreferences.PrefValueEncryptionScheme.AES256_GCM) - .edit() - .putString("name", name) // Use the shared preferences and editor as you normally would - .putString("password", password); - - editor.commit(); - } -} diff --git a/java/ql/test/experimental/query-tests/security/CWE-312/CleartextStorageSharedPrefs.qlref b/java/ql/test/experimental/query-tests/security/CWE-312/CleartextStorageSharedPrefs.qlref deleted file mode 100644 index 9319c78be055..000000000000 --- a/java/ql/test/experimental/query-tests/security/CWE-312/CleartextStorageSharedPrefs.qlref +++ /dev/null @@ -1 +0,0 @@ -experimental/Security/CWE/CWE-312/CleartextStorageSharedPrefs.ql diff --git a/java/ql/test/query-tests/security/CWE-312/CleartextStorageSharedPrefsTest.expected b/java/ql/test/query-tests/security/CWE-312/CleartextStorageSharedPrefsTest.expected new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/java/ql/test/query-tests/security/CWE-312/CleartextStorageSharedPrefsTest.java b/java/ql/test/query-tests/security/CWE-312/CleartextStorageSharedPrefsTest.java new file mode 100644 index 000000000000..1b6d8a8c3a40 --- /dev/null +++ b/java/ql/test/query-tests/security/CWE-312/CleartextStorageSharedPrefsTest.java @@ -0,0 +1,97 @@ +import android.app.Activity; +import android.content.Context; +import android.content.SharedPreferences; +import android.content.SharedPreferences.Editor; +import androidx.security.crypto.MasterKey; +import androidx.security.crypto.EncryptedSharedPreferences; +import java.nio.charset.StandardCharsets; +import java.util.Base64; +import java.security.MessageDigest; + +public class CleartextStorageSharedPrefsTest extends Activity { + public void testSetSharedPrefs1(Context context, String name, String password) { + SharedPreferences sharedPrefs = + context.getSharedPreferences("user_prefs", Context.MODE_PRIVATE); + Editor editor = sharedPrefs.edit(); + editor.putString("name", name); // Safe + editor.putString("password", password); // $ hasCleartextStorageSharedPrefs + editor.commit(); + } + + public void testSetSharedPrefs2(Context context, String name, String password) + throws Exception { + SharedPreferences sharedPrefs = + context.getSharedPreferences("user_prefs", Context.MODE_PRIVATE); + Editor editor = sharedPrefs.edit(); + editor.putString("name", encrypt(name)); // Safe + editor.putString("password", encrypt(password)); // Safe + editor.commit(); + } + + private static String encrypt(String cleartext) throws Exception { + MessageDigest digest = MessageDigest.getInstance("SHA-256"); + byte[] hash = digest.digest(cleartext.getBytes(StandardCharsets.UTF_8)); + String encoded = Base64.getEncoder().encodeToString(hash); + return encoded; + } + + public void testSetSharedPrefs3(Context context, String name, String password) + throws Exception { + String encUsername = encrypt(name); + String encPassword = encrypt(password); + SharedPreferences sharedPrefs = + context.getSharedPreferences("user_prefs", Context.MODE_PRIVATE); + Editor editor = sharedPrefs.edit(); + editor.putString("name", encUsername); // Safe + editor.putString("password", encPassword); // Safe + editor.commit(); + } + + public void testSetSharedPrefs4(Context context, String name, String password) + throws Exception { + MasterKey masterKey = new MasterKey.Builder(context, MasterKey.DEFAULT_MASTER_KEY_ALIAS) + .setKeyScheme(MasterKey.KeyScheme.AES256_GCM).build(); + + SharedPreferences sharedPreferences = + EncryptedSharedPreferences.create(context, "secret_shared_prefs", masterKey, + EncryptedSharedPreferences.PrefKeyEncryptionScheme.AES256_SIV, + EncryptedSharedPreferences.PrefValueEncryptionScheme.AES256_GCM); + + SharedPreferences.Editor editor = sharedPreferences.edit(); + editor.putString("name", name); // Safe + editor.putString("password", password); // Safe + editor.commit(); + } + + public void testSetSharedPrefs5(Context context, String name, String password) + throws Exception { + MasterKey masterKey = new MasterKey.Builder(context, MasterKey.DEFAULT_MASTER_KEY_ALIAS) + .setKeyScheme(MasterKey.KeyScheme.AES256_GCM).build(); + + SharedPreferences.Editor editor = + EncryptedSharedPreferences + .create(context, "secret_shared_prefs", masterKey, + EncryptedSharedPreferences.PrefKeyEncryptionScheme.AES256_SIV, + EncryptedSharedPreferences.PrefValueEncryptionScheme.AES256_GCM) + .edit(); + + editor.putString("name", name); // Safe + editor.putString("password", password); // Safe + editor.commit(); + } + + public void testSetSharedPrefs6(Context context, String name, String password) + throws Exception { + MasterKey masterKey = new MasterKey.Builder(context, MasterKey.DEFAULT_MASTER_KEY_ALIAS) + .setKeyScheme(MasterKey.KeyScheme.AES256_GCM).build(); + + SharedPreferences.Editor editor = EncryptedSharedPreferences + .create(context, "secret_shared_prefs", masterKey, + EncryptedSharedPreferences.PrefKeyEncryptionScheme.AES256_SIV, + EncryptedSharedPreferences.PrefValueEncryptionScheme.AES256_GCM) + .edit().putString("name", name) /// Safe + .putString("password", password); // Safe + + editor.commit(); + } +} diff --git a/java/ql/test/query-tests/security/CWE-312/CleartextStorageSharedPrefsTest.ql b/java/ql/test/query-tests/security/CWE-312/CleartextStorageSharedPrefsTest.ql new file mode 100644 index 000000000000..cbd17429e5c7 --- /dev/null +++ b/java/ql/test/query-tests/security/CWE-312/CleartextStorageSharedPrefsTest.ql @@ -0,0 +1,22 @@ +import java +import semmle.code.java.security.CleartextStorageSharedPrefsQuery +import TestUtilities.InlineExpectationsTest + +class CleartextStorageSharedPrefsTest extends InlineExpectationsTest { + CleartextStorageSharedPrefsTest() { this = "CleartextStorageSharedPrefsTest" } + + override string getARelevantTag() { result = "hasCleartextStorageSharedPrefs" } + + override predicate hasActualResult(Location location, string element, string tag, string value) { + tag = "hasCleartextStorageSharedPrefs" and + exists(SensitiveSource data, SharedPreferencesEditorMethodAccess s, Expr input, Expr store | + input = s.getAnInput() and + store = s.getAStore() and + data.flowsTo(input) + | + input.getLocation() = location and + element = input.toString() and + value = "" + ) + } +} diff --git a/java/ql/test/experimental/query-tests/security/CWE-312/options b/java/ql/test/query-tests/security/CWE-312/options similarity index 66% rename from java/ql/test/experimental/query-tests/security/CWE-312/options rename to java/ql/test/query-tests/security/CWE-312/options index 43e25f608b67..f017f81ff2fe 100644 --- a/java/ql/test/experimental/query-tests/security/CWE-312/options +++ b/java/ql/test/query-tests/security/CWE-312/options @@ -1 +1 @@ -// semmle-extractor-options: --javac-args -cp ${testdir}/../../../../stubs/google-android-9.0.0 +// semmle-extractor-options: --javac-args -cp ${testdir}/../../../stubs/google-android-9.0.0