-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Java: Create new query Cleartext storage of sensitive information in Android databases #6492
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
atorralba
merged 13 commits into
github:main
from
atorralba:atorralba/android-cleartext-storage-database
Feb 2, 2022
Merged
Changes from all commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
f0604e2
Added query for Cleartext Storage in Android Database
atorralba 16b61f7
Fix QLDocs and the qhelp example
atorralba ee84dae
Fix predicate name
atorralba c5ed5fc
Apply suggestions from code review
atorralba 4e4f619
Update java/ql/lib/semmle/code/java/security/CleartextStorageAndroidD…
atorralba baa1f71
Add QLDoc
atorralba 5cf6644
Remove unneeded nonSuspicious values
atorralba 652a1d2
Fix wrongly resolved rebase conflicts
atorralba 4f25359
Fix method name in LocalDatabaseOpenMethodAccess
atorralba c6dd7dd
Fix stub
atorralba 4df0f39
Move ContentProvider models to the appropriate file
atorralba 4f4f531
Add missing QLDoc
atorralba 54e8ea5
Apply suggestions from code review
atorralba 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
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
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
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
59 changes: 59 additions & 0 deletions
59
java/ql/lib/semmle/code/java/frameworks/android/ContentProviders.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,59 @@ | ||
| /** | ||
| * Provides classes and predicates for working with Content Providers. | ||
| */ | ||
|
|
||
| import java | ||
| import semmle.code.java.dataflow.ExternalFlow | ||
|
|
||
| /** The class `android.content.ContentValues`. */ | ||
| class ContentValues extends Class { | ||
| ContentValues() { this.hasQualifiedName("android.content", "ContentValues") } | ||
| } | ||
|
|
||
| private class ContentProviderSourceModels extends SourceModelCsv { | ||
| override predicate row(string row) { | ||
| row = | ||
| [ | ||
| // ContentInterface models are here for backwards compatibility (it was removed in API 28) | ||
| "android.content;ContentInterface;true;call;(String,String,String,Bundle);;Parameter[0..3];contentprovider", | ||
| "android.content;ContentProvider;true;call;(String,String,String,Bundle);;Parameter[0..3];contentprovider", | ||
| "android.content;ContentProvider;true;call;(String,String,Bundle);;Parameter[0..2];contentprovider", | ||
| "android.content;ContentProvider;true;delete;(Uri,String,String[]);;Parameter[0..2];contentprovider", | ||
| "android.content;ContentInterface;true;delete;(Uri,Bundle);;Parameter[0..1];contentprovider", | ||
| "android.content;ContentProvider;true;delete;(Uri,Bundle);;Parameter[0..1];contentprovider", | ||
| "android.content;ContentInterface;true;getType;(Uri);;Parameter[0];contentprovider", | ||
| "android.content;ContentProvider;true;getType;(Uri);;Parameter[0];contentprovider", | ||
| "android.content;ContentInterface;true;insert;(Uri,ContentValues,Bundle);;Parameter[0];contentprovider", | ||
| "android.content;ContentProvider;true;insert;(Uri,ContentValues,Bundle);;Parameter[0..2];contentprovider", | ||
| "android.content;ContentProvider;true;insert;(Uri,ContentValues);;Parameter[0..1];contentprovider", | ||
| "android.content;ContentInterface;true;openAssetFile;(Uri,String,CancellationSignal);;Parameter[0];contentprovider", | ||
| "android.content;ContentProvider;true;openAssetFile;(Uri,String,CancellationSignal);;Parameter[0];contentprovider", | ||
| "android.content;ContentProvider;true;openAssetFile;(Uri,String);;Parameter[0];contentprovider", | ||
| "android.content;ContentInterface;true;openTypedAssetFile;(Uri,String,Bundle,CancellationSignal);;Parameter[0..2];contentprovider", | ||
| "android.content;ContentProvider;true;openTypedAssetFile;(Uri,String,Bundle,CancellationSignal);;Parameter[0..2];contentprovider", | ||
| "android.content;ContentProvider;true;openTypedAssetFile;(Uri,String,Bundle);;Parameter[0..2];contentprovider", | ||
| "android.content;ContentInterface;true;openFile;(Uri,String,CancellationSignal);;Parameter[0];contentprovider", | ||
| "android.content;ContentProvider;true;openFile;(Uri,String,CancellationSignal);;Parameter[0];contentprovider", | ||
| "android.content;ContentProvider;true;openFile;(Uri,String);;Parameter[0];contentprovider", | ||
| "android.content;ContentInterface;true;query;(Uri,String[],Bundle,CancellationSignal);;Parameter[0..2];contentprovider", | ||
| "android.content;ContentProvider;true;query;(Uri,String[],Bundle,CancellationSignal);;Parameter[0..2];contentprovider", | ||
| "android.content;ContentProvider;true;query;(Uri,String[],String,String[],String);;Parameter[0..4];contentprovider", | ||
| "android.content;ContentProvider;true;query;(Uri,String[],String,String[],String,CancellationSignal);;Parameter[0..4];contentprovider", | ||
| "android.content;ContentInterface;true;update;(Uri,ContentValues,Bundle);;Parameter[0..2];contentprovider", | ||
| "android.content;ContentProvider;true;update;(Uri,ContentValues,Bundle);;Parameter[0..2];contentprovider", | ||
| "android.content;ContentProvider;true;update;(Uri,ContentValues,String,String[]);;Parameter[0..3];contentprovider" | ||
| ] | ||
| } | ||
| } | ||
|
|
||
| private class SummaryModels extends SummaryModelCsv { | ||
| override predicate row(string row) { | ||
| row = | ||
| [ | ||
| "android.content;ContentValues;false;put;;;Argument[0];MapKey of Argument[-1];value", | ||
| "android.content;ContentValues;false;put;;;Argument[1];MapValue of Argument[-1];value", | ||
| "android.content;ContentValues;false;putAll;;;MapKey of Argument[0];MapKey of Argument[-1];value", | ||
| "android.content;ContentValues;false;putAll;;;MapValue of Argument[0];MapValue of Argument[-1];value" | ||
| ] | ||
| } | ||
| } |
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
23 changes: 23 additions & 0 deletions
23
java/ql/lib/semmle/code/java/frameworks/android/Widget.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,23 @@ | ||
| /** Provides classes and predicates for working with Android widgets. */ | ||
|
|
||
| import java | ||
| import semmle.code.java.dataflow.ExternalFlow | ||
| import semmle.code.java.dataflow.FlowSources | ||
|
|
||
| private class AndroidWidgetSourceModels extends SourceModelCsv { | ||
| override predicate row(string row) { | ||
| row = "android.widget;EditText;true;getText;;;ReturnValue;android-widget" | ||
| } | ||
| } | ||
|
|
||
| private class DefaultAndroidWidgetSources extends RemoteFlowSource { | ||
| DefaultAndroidWidgetSources() { sourceNode(this, "android-widget") } | ||
|
|
||
| override string getSourceType() { result = "Android widget source" } | ||
| } | ||
|
|
||
| private class AndroidWidgetSummaryModels extends SummaryModelCsv { | ||
| override predicate row(string row) { | ||
| row = "android.widget;EditText;true;getText;;;Argument[-1];ReturnValue;taint" | ||
| } | ||
| } |
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
131 changes: 131 additions & 0 deletions
131
java/ql/lib/semmle/code/java/security/CleartextStorageAndroidDatabaseQuery.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,131 @@ | ||
| /** Provides classes and predicates to reason about cleartext storage in Android databases. */ | ||
|
|
||
| import java | ||
| import semmle.code.java.dataflow.DataFlow | ||
| import semmle.code.java.frameworks.android.ContentProviders | ||
| import semmle.code.java.frameworks.android.Intent | ||
| import semmle.code.java.frameworks.android.SQLite | ||
| import semmle.code.java.security.CleartextStorageQuery | ||
|
|
||
| private class LocalDatabaseCleartextStorageSink extends CleartextStorageSink { | ||
| LocalDatabaseCleartextStorageSink() { localDatabaseInput(_, this.asExpr()) } | ||
| } | ||
|
|
||
| private class LocalDatabaseCleartextStorageStep extends CleartextStorageAdditionalTaintStep { | ||
| override predicate step(DataFlow::Node n1, DataFlow::Node n2) { | ||
| // EditText.getText() return type is parsed as `Object`, so we need to | ||
| // add a taint step for `Object.toString` to model `editText.getText().toString()` | ||
| exists(MethodAccess ma, Method m | | ||
| ma.getMethod() = m and | ||
| m.getDeclaringType() instanceof TypeObject and | ||
| m.hasName("toString") | ||
| | | ||
| n1.asExpr() = ma.getQualifier() and n2.asExpr() = ma | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| /** The creation of an object that can be used to store data in a local database. */ | ||
| class LocalDatabaseOpenMethodAccess extends Storable, Call { | ||
| LocalDatabaseOpenMethodAccess() { | ||
| exists(Method m | this.(MethodAccess).getMethod() = m | | ||
| m.getDeclaringType().getASupertype*() instanceof TypeSQLiteOpenHelper and | ||
| m.hasName("getWritableDatabase") | ||
| or | ||
| m.getDeclaringType() instanceof TypeSQLiteDatabase and | ||
| m.hasName(["create", "openDatabase", "openOrCreateDatabase", "compileStatement"]) | ||
| or | ||
| m.getDeclaringType().getASupertype*() instanceof TypeContext and | ||
| m.hasName("openOrCreateDatabase") | ||
| ) | ||
| or | ||
| this.(ClassInstanceExpr).getConstructedType() instanceof ContentValues | ||
| } | ||
|
|
||
| override Expr getAnInput() { | ||
| exists(LocalDatabaseFlowConfig config, DataFlow::Node database | | ||
| localDatabaseInput(database, result) and | ||
| config.hasFlow(DataFlow::exprNode(this), database) | ||
| ) | ||
| } | ||
|
|
||
| override Expr getAStore() { | ||
| exists(LocalDatabaseFlowConfig config, DataFlow::Node database | | ||
| localDatabaseStore(database, result) and | ||
| config.hasFlow(DataFlow::exprNode(this), database) | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| /** A method that is both a database input and a database store. */ | ||
| private class LocalDatabaseInputStoreMethod extends Method { | ||
| LocalDatabaseInputStoreMethod() { | ||
| this.getDeclaringType() instanceof TypeSQLiteDatabase and | ||
| this.getName().matches("exec%SQL") | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Holds if `input` is a value being prepared for being stored into the SQLite dataabse `database`. | ||
| * This can be done using prepared statements, using the class `ContentValues`, or directly | ||
| * appending `input` to a SQL query. | ||
| */ | ||
| private predicate localDatabaseInput(DataFlow::Node database, Argument input) { | ||
| exists(Method m | input.getCall().getCallee() = m | | ||
| m instanceof LocalDatabaseInputStoreMethod and | ||
| database.asExpr() = input.getCall().getQualifier() | ||
| or | ||
| m.getDeclaringType() instanceof TypeSQLiteDatabase and | ||
| m.hasName("compileStatement") and | ||
| database.asExpr() = input.getCall() | ||
| or | ||
| m.getDeclaringType() instanceof ContentValues and | ||
| m.hasName("put") and | ||
| input.getPosition() = 1 and | ||
| database.asExpr() = input.getCall().getQualifier() | ||
| ) | ||
| } | ||
|
|
||
| /** | ||
| * Holds if `store` is a method call for storing a previously appended input value, | ||
| * either through the use of prepared statements, via the `ContentValues` class, or | ||
| * directly executing a raw SQL query. | ||
| */ | ||
| private predicate localDatabaseStore(DataFlow::Node database, MethodAccess store) { | ||
| exists(Method m | store.getMethod() = m | | ||
| m instanceof LocalDatabaseInputStoreMethod and | ||
| database.asExpr() = store.getQualifier() | ||
| or | ||
| m.getDeclaringType() instanceof TypeSQLiteDatabase and | ||
| m.getName().matches(["insert%", "replace%", "update%"]) and | ||
| database.asExpr() = store.getAnArgument() and | ||
| database.getType() instanceof ContentValues | ||
| or | ||
| m.getDeclaringType() instanceof TypeSQLiteStatement and | ||
| m.hasName(["executeInsert", "executeUpdateDelete"]) and | ||
| database.asExpr() = store.getQualifier() | ||
| ) | ||
| } | ||
|
|
||
| private class LocalDatabaseFlowConfig extends DataFlow::Configuration { | ||
| LocalDatabaseFlowConfig() { this = "LocalDatabaseFlowConfig" } | ||
|
|
||
| override predicate isSource(DataFlow::Node source) { | ||
| source.asExpr() instanceof LocalDatabaseOpenMethodAccess | ||
| } | ||
|
|
||
| override predicate isSink(DataFlow::Node sink) { | ||
| localDatabaseInput(sink, _) or | ||
| localDatabaseStore(sink, _) | ||
| } | ||
|
|
||
| override predicate isAdditionalFlowStep(DataFlow::Node n1, DataFlow::Node n2) { | ||
| // Adds a step for tracking databases through field flow, that is, a database is opened and | ||
| // assigned to a field, and then an input or store method is called on that field elsewhere. | ||
| exists(Field f | | ||
| f.getType() instanceof TypeSQLiteDatabase and | ||
| f.getAnAssignedValue() = n1.asExpr() and | ||
| f = n2.asExpr().(FieldRead).getField() | ||
atorralba marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| ) | ||
| } | ||
| } | ||
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
27 changes: 27 additions & 0 deletions
27
java/ql/src/Security/CWE/CWE-312/CleartextStorageAndroidDatabase.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,27 @@ | ||
| public void sqliteStorageUnsafe(Context ctx, String name, String password) { | ||
| // BAD - sensitive information saved in cleartext. | ||
| SQLiteDatabase db = ctx.openOrCreateDatabase("test", Context.MODE_PRIVATE, null); | ||
| db.execSQL("INSERT INTO users VALUES (?, ?)", new String[] {name, password}); | ||
| } | ||
|
|
||
| public void sqliteStorageSafe(Context ctx, String name, String password) { | ||
| // GOOD - sensitive information encrypted with a custom method. | ||
| SQLiteDatabase db = ctx.openOrCreateDatabase("test", Context.MODE_PRIVATE, null); | ||
| db.execSQL("INSERT INTO users VALUES (?, ?)", new String[] {name, encrypt(password)}); | ||
| } | ||
|
|
||
| public void sqlCipherStorageSafe(String name, String password, String databasePassword) { | ||
| // GOOD - sensitive information saved using SQLCipher. | ||
| net.sqlcipher.database.SQLiteDatabase db = | ||
| net.sqlcipher.database.SQLiteDatabase.openOrCreateDatabase("test", databasePassword, null); | ||
| db.execSQL("INSERT INTO users VALUES (?, ?)", new String[] {name, password}); | ||
| } | ||
|
|
||
| private static String encrypt(String cleartext) { | ||
| // Use an encryption or strong hashing algorithm in the real world. | ||
| // The example below just returns a SHA-256 hash. | ||
| MessageDigest digest = MessageDigest.getInstance("SHA-256"); | ||
| byte[] hash = digest.digest(cleartext.getBytes(StandardCharsets.UTF_8)); | ||
| String encoded = Base64.getEncoder().encodeToString(hash); | ||
| return encoded; | ||
| } |
36 changes: 36 additions & 0 deletions
36
java/ql/src/Security/CWE/CWE-312/CleartextStorageAndroidDatabase.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,36 @@ | ||
| <!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd"> | ||
| <qhelp> | ||
| <overview> | ||
| <p> | ||
| SQLite is a lightweight database engine commonly used in Android devices to store data. By itself, SQLite does not offer any encryption mechanism by default and stores all data in cleartext, which introduces a risk if sensitive data like credentials, authentication tokens or personal identifiable information (PII) are directly stored in a SQLite database. The information could 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> | ||
|
|
||
| <recommendation> | ||
| <p> | ||
| Use <code>SQLCipher</code> or similar libraries to add encryption capabilities to SQLite. Alternatively, encrypt sensitive data using cryptographically secure algorithms before storing it in the database. | ||
| </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 database. | ||
| </p> | ||
| <sample src="CleartextStorageAndroidDatabase.java" /> | ||
| </example> | ||
|
|
||
| <references> | ||
| <li> | ||
| Android Developers: | ||
| <a href="https://developer.android.com/topic/security/data">Work with data more securely</a> | ||
| </li> | ||
| <li> | ||
| SQLCipher: | ||
| <a href="https://www.zetetic.net/sqlcipher/sqlcipher-for-android/">Android Application Integration</a> | ||
| </li> | ||
| </references> | ||
| </qhelp> |
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.