Skip to content
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
public void testSetSharedPrefs(Context context, String name, String password)
{
{
// BAD - save sensitive information in cleartext
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
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();
}

{
// GOOD - save sensitive information 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();

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);
editor.putString("password", password);
editor.commit();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
<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.
</p>
</overview>

<recommendation>
<p>
Use the <code>EncryptedSharedPreferences</code> API or other encryption algorithms for storing sensitive information.
</p>
</recommendation>

<example>
<p>
In the first example, sensitive user information is stored in cleartext.
</p>

<p>
In the second and third examples, the code encrypts sensitive information before saving it to the device.
</p>
<sample src="CleartextStorageSharedPrefs.java" />
</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>
</li>
<li>
ProAndroidDev:
<a href="https://proandroiddev.com/encrypted-preferences-in-android-af57a89af7c8">Encrypted Preferences in Android</a>
</li>
</references>
</qhelp>
Original file line number Diff line number Diff line change
@@ -0,0 +1,163 @@
/**
* @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
* @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 SharedPreferences::SetPreferenceMethod 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 SharedPreferences::StorePreferenceMethod 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 SharedPreferencesEditor
}

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() {
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 SharedPreferences::SetPreferenceMethod 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 SharedPreferences::CreateEncryptedMethod
}

override predicate isSink(DataFlow4::Node sink) {
sink.asExpr().getType() instanceof SharedPreferences::TypeBase
}
}

/** The call to get a `SharedPreferences.Editor` object, which can set shared preferences or be stored to device. */
class SharedPreferencesEditor extends MethodAccess {
SharedPreferencesEditor() {
this.getMethod() instanceof SharedPreferences::GetEditorMethod 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 SharedPreferences::SetPreferenceMethod 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, SharedPreferencesEditor 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"
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
/** Provides classes related to `android.content.SharedPreferences`. */

import java
Comment thread
luchua-bc marked this conversation as resolved.

/** Definitions related to `android.content.SharedPreferences`. */
module SharedPreferences {
Comment thread
luchua-bc marked this conversation as resolved.
Outdated
/** The interface `android.content.SharedPreferences` */
Comment thread
luchua-bc marked this conversation as resolved.
Outdated
class TypeBase extends Interface {
TypeBase() { hasQualifiedName("android.content", "SharedPreferences") }
Comment thread
luchua-bc marked this conversation as resolved.
Outdated
}

/** The class `androidx.security.crypto.EncryptedSharedPreferences`, which implements `SharedPreferences` with encryption support. */
class TypeEncrypted extends Class {
TypeEncrypted() { hasQualifiedName("androidx.security.crypto", "EncryptedSharedPreferences") }
Comment thread
luchua-bc marked this conversation as resolved.
Outdated
}

/** The create method of `androidx.security.crypto.EncryptedSharedPreferences` */
Comment thread
luchua-bc marked this conversation as resolved.
Outdated
class CreateEncryptedMethod extends Method {
CreateEncryptedMethod() {
getDeclaringType() instanceof TypeEncrypted and
hasName("create")
Comment thread
luchua-bc marked this conversation as resolved.
Outdated
}
}

/** A getter method of `android.content.SharedPreferences`. */
class GetPreferenceMethod extends Method {
GetPreferenceMethod() {
getDeclaringType() instanceof TypeBase and
getName().matches("get%")
}
}
Comment thread
luchua-bc marked this conversation as resolved.
Outdated

/** Returns `android.content.SharedPreferences.Editor` from the `edit` call of `android.content.SharedPreferences`. */
class GetEditorMethod extends Method {
GetEditorMethod() {
getDeclaringType() instanceof TypeBase and
hasName("edit") and
getReturnType() instanceof TypeEditor
}
}

/** Definitions related to `android.content.SharedPreferences.Editor`. */
Comment thread
luchua-bc marked this conversation as resolved.
Outdated
class TypeEditor extends Interface {
TypeEditor() { hasQualifiedName("android.content", "SharedPreferences$Editor") }
Comment thread
luchua-bc marked this conversation as resolved.
Outdated
}

/** A setter method for `android.content.SharedPreferences`. */
Comment thread
luchua-bc marked this conversation as resolved.
Outdated
class SetPreferenceMethod extends Method {
SetPreferenceMethod() {
Comment thread
luchua-bc marked this conversation as resolved.
Outdated
getDeclaringType() instanceof TypeEditor and
getName().matches("put%")
}
}

/** A setter method for `android.content.SharedPreferences`. */
Comment thread
luchua-bc marked this conversation as resolved.
Outdated
class StorePreferenceMethod extends Method {
StorePreferenceMethod() {
getDeclaringType() instanceof TypeEditor and
hasName(["commit", "apply"])
}
}
}
Comment thread
luchua-bc marked this conversation as resolved.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
| 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 |
Loading