Merge pull request #6468 from atorralba/atorralba/promote-cleartext-sharedprefs

Java: Promote Cleartext storage of sensitive information using SharedPreferences from experimental
This commit is contained in:
Tony Torralba
2022-01-11 16:23:53 +01:00
committed by GitHub
13 changed files with 239 additions and 287 deletions

View File

@@ -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, _)
}
}

View File

@@ -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;
}

View File

@@ -2,7 +2,7 @@
<qhelp>
<overview>
<p>
<code>SharedPreferences</code> 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.
<code>SharedPreferences</code> 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.
</p>
</overview>
@@ -24,10 +24,6 @@
</example>
<references>
<li>
CWE:
<a href="https://cwe.mitre.org/data/definitions/312.html">CWE-312: Cleartext Storage of Sensitive Information</a>
</li>
<li>
Android Developers:
<a href="https://developer.android.com/topic/security/data">Work with data more securely</a>

View File

@@ -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"

View File

@@ -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).

View File

@@ -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"

View File

@@ -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 |

View File

@@ -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();
}
}

View File

@@ -1 +0,0 @@
experimental/Security/CWE/CWE-312/CleartextStorageSharedPrefs.ql

View File

@@ -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();
}
}

View File

@@ -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 = ""
)
}
}

View File

@@ -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