-
Notifications
You must be signed in to change notification settings - Fork 2k
Java: Query to detect cleartext storage of sensitive information using Android SharedPreferences #4675
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
aschackmull
merged 11 commits into
github:main
from
luchua-bc:cleartext-storage-shared-prefs
Jan 8, 2021
Merged
Java: Query to detect cleartext storage of sensitive information using Android SharedPreferences #4675
Changes from 9 commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
0bd6255
Query for cleartext storage using Android SharedPreferences
luchua-bc 85434ca
Format the source code and update qldoc
luchua-bc a311462
Move to query-test folder and update qldoc
luchua-bc a491604
Enhance the query and add more test cases
luchua-bc 7ad031c
Move to experimental and update qldoc
luchua-bc a83ddd6
Add comments about how the future promotion should go
luchua-bc ad0ac5b
Change kind to problem
luchua-bc 5690bf4
Optimize the query
luchua-bc f13b881
Update class/method names in the module
luchua-bc b54e5b1
Revamp the library module
luchua-bc 606d094
Update qldoc
luchua-bc File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
39 changes: 39 additions & 0 deletions
39
java/ql/src/experimental/Security/CWE/CWE-312/CleartextStorageSharedPrefs.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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(); | ||
| } | ||
| } |
40 changes: 40 additions & 0 deletions
40
java/ql/src/experimental/Security/CWE/CWE-312/CleartextStorageSharedPrefs.qhelp
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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> |
163 changes: 163 additions & 0 deletions
163
java/ql/src/experimental/Security/CWE/CWE-312/CleartextStorageSharedPrefs.ql
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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" |
62 changes: 62 additions & 0 deletions
62
java/ql/src/semmle/code/java/frameworks/android/SharedPreferences.qll
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| /** Provides classes related to `android.content.SharedPreferences`. */ | ||
|
|
||
| import java | ||
|
|
||
| /** Definitions related to `android.content.SharedPreferences`. */ | ||
| module SharedPreferences { | ||
|
luchua-bc marked this conversation as resolved.
Outdated
|
||
| /** The interface `android.content.SharedPreferences` */ | ||
|
luchua-bc marked this conversation as resolved.
Outdated
|
||
| class TypeBase extends Interface { | ||
| TypeBase() { hasQualifiedName("android.content", "SharedPreferences") } | ||
|
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") } | ||
|
luchua-bc marked this conversation as resolved.
Outdated
|
||
| } | ||
|
|
||
| /** The create method of `androidx.security.crypto.EncryptedSharedPreferences` */ | ||
|
luchua-bc marked this conversation as resolved.
Outdated
|
||
| class CreateEncryptedMethod extends Method { | ||
| CreateEncryptedMethod() { | ||
| getDeclaringType() instanceof TypeEncrypted and | ||
| hasName("create") | ||
|
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%") | ||
| } | ||
| } | ||
|
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`. */ | ||
|
luchua-bc marked this conversation as resolved.
Outdated
|
||
| class TypeEditor extends Interface { | ||
| TypeEditor() { hasQualifiedName("android.content", "SharedPreferences$Editor") } | ||
|
luchua-bc marked this conversation as resolved.
Outdated
|
||
| } | ||
|
|
||
| /** A setter method for `android.content.SharedPreferences`. */ | ||
|
luchua-bc marked this conversation as resolved.
Outdated
|
||
| class SetPreferenceMethod extends Method { | ||
| SetPreferenceMethod() { | ||
|
luchua-bc marked this conversation as resolved.
Outdated
|
||
| getDeclaringType() instanceof TypeEditor and | ||
| getName().matches("put%") | ||
| } | ||
| } | ||
|
|
||
| /** A setter method for `android.content.SharedPreferences`. */ | ||
|
luchua-bc marked this conversation as resolved.
Outdated
|
||
| class StorePreferenceMethod extends Method { | ||
| StorePreferenceMethod() { | ||
| getDeclaringType() instanceof TypeEditor and | ||
| hasName(["commit", "apply"]) | ||
| } | ||
| } | ||
| } | ||
|
luchua-bc marked this conversation as resolved.
|
||
1 change: 1 addition & 0 deletions
1
java/ql/test/experimental/query-tests/security/CWE-312/CleartextStorageSharedPrefs.expected
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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 | |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.