-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Add Tests and Demos about "Sensitive Data Stored Unencrypted via SQLite" #3534
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
base: master
Are you sure you want to change the base?
Changes from all commits
6bc899e
dbc2643
640c3f2
2e1c845
326e445
d9fc682
5dd5634
f413d42
4a53c5e
de2f3e2
6cd521d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,99 @@ | ||||||||||
| --- | ||||||||||
| title: Encrypt Sensitive Data in Private Storage Locations | ||||||||||
| alias: encrypt-sensitive-data | ||||||||||
| id: MASTG-BEST-0025 | ||||||||||
| platform: android | ||||||||||
| knowledge: [MASTG-KNOW-0036, MASTG-KNOW-0037, MASTG-KNOW-0038, MASTG-KNOW-0041] | ||||||||||
| --- | ||||||||||
|
|
||||||||||
| Limit local storage of sensitive data to cases where it is strictly required. Android's app-private directories provide isolation but do not protect data on rooted devices, compromised devices, or during physical extraction. Therefore, sensitive data stored locally must always be **encrypted at rest**. | ||||||||||
|
|
||||||||||
| ## App Private Storage | ||||||||||
|
|
||||||||||
| The following sandboxed locations (`/data/data/<package>/`) are private but not secure against privileged attackers: | ||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
|
|
||||||||||
| - `filesDir` – Internal app files. | ||||||||||
| - `noBackupDir` – Internal files excluded from Auto Backup. | ||||||||||
| - `databases` – SQLite databases. | ||||||||||
| - `shared_prefs` – SharedPreferences. | ||||||||||
|
|
||||||||||
| Because sandbox isolation does not mitigate elevated access, **plaintext sensitive data must not be stored** in these locations. | ||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
|
|
||||||||||
| ## Use Platform Cryptography | ||||||||||
|
|
||||||||||
| Prefer platform-backed cryptography for local storage encryption. | ||||||||||
|
|
||||||||||
| ### Deprecated Jetpack Security Components | ||||||||||
|
|
||||||||||
| **`EncryptedFile`** and **`EncryptedSharedPreferences`** (from `androidx.security:security-crypto`) are **deprecated** as of 1.1.0. | ||||||||||
|
|
||||||||||
| #### `EncryptedFile` | ||||||||||
|
|
||||||||||
| - **Deprecated:** API documentation states: "Use `java.io.File` instead." | ||||||||||
| - Uses a `MasterKey` stored in Android Keystore and enforces **AES256\_GCM\_HKDF\_4KB**. | ||||||||||
| - Must be **excluded from Auto Backup** (the restored file will not decrypt because the key will differ). | ||||||||||
| - A hidden keyset preferences file (`.../shared_prefs/__androidx_security_crypto_encrypted_file_pref__`) is auto-created and must also be **excluded from backups**. | ||||||||||
| - Decryption requires the identical master key and keyset; mismatch causes `GeneralSecurityException`. | ||||||||||
|
|
||||||||||
| #### `EncryptedSharedPreferences` | ||||||||||
|
|
||||||||||
| - **Deprecated:** API documentation states: "Use `android.content.SharedPreferences` instead." | ||||||||||
| - Encrypts keys (AES-SIV) and values (AES-GCM). | ||||||||||
| - Must be **excluded from Auto Backup.** | ||||||||||
| - Requires the same master key and keyset; mismatch leads to decryption failure. | ||||||||||
|
|
||||||||||
| ### When Using Deprecated Classes | ||||||||||
|
|
||||||||||
| - Exclude encrypted files and all keyset preference files from Auto Backup. | ||||||||||
| - Reuse a single, consistent **`MasterKey`**. | ||||||||||
| - Prefer **hardware-backed Keystore keys** (TEE/StrongBox) when available. | ||||||||||
|
|
||||||||||
| ### For New Codebases | ||||||||||
|
|
||||||||||
| Use maintained and recommended alternatives: | ||||||||||
|
|
||||||||||
| - Android Keystore + manual **AES-GCM** implementation. | ||||||||||
| - **Tink**, **SQLCipher**, or equivalent well-maintained cryptographic library. | ||||||||||
|
|
||||||||||
| ## Envelope Encryption (Recommended) | ||||||||||
|
|
||||||||||
| For databases or large files, use **envelope encryption**: | ||||||||||
|
|
||||||||||
| - **DEK (Data Encryption Key)** — symmetric key (AES-GCM) used to encrypt data. | ||||||||||
| - **KEK (Key Encryption Key)** — stored securely in Android Keystore; used to encrypt the DEK. | ||||||||||
|
|
||||||||||
| This approach provides efficient symmetric encryption while protecting the key material using hardware-backed Keystore, when available. | ||||||||||
|
|
||||||||||
| ### SQLite | ||||||||||
|
|
||||||||||
| - Use **SQLCipher** for full-database AES-256 encryption. | ||||||||||
| - Protect the SQLCipher passphrase using **envelope encryption**. | ||||||||||
| - Never store database keys or passphrases in plaintext `SharedPreferences`. | ||||||||||
|
|
||||||||||
| ### Key–Value Data | ||||||||||
|
|
||||||||||
| - Use `EncryptedSharedPreferences` only for legacy/maintenance code. | ||||||||||
| - Prefer **DataStore** + manual **AES-GCM** or **Tink** for new implementations. | ||||||||||
| - Ensure **AES-GCM** operations use unique nonces. | ||||||||||
|
|
||||||||||
| ## Storage Location Guidance | ||||||||||
|
|
||||||||||
| | Location | Recommendation | | ||||||||||
| | :--- | :--- | | ||||||||||
| | `filesDir` | Allowed only for **encrypted data**. Private, but unsecured on rooted devices or after physical extraction. | | ||||||||||
| | `noBackupFilesDir` | Allowed for **encrypted data that must not appear in backups**. Prevents key mismatch issues after system restore. | | ||||||||||
|
|
||||||||||
| ### Locations to Avoid | ||||||||||
|
|
||||||||||
| - **External / Shared Storage:** Prohibited. No protection against access by other apps. | ||||||||||
| - **Plaintext `SharedPreferences`:** Prohibited. Data is easily accessible if the device is compromised. | ||||||||||
| - **Plaintext Cache:** Prohibited. Lacks strong protection for sensitive data. | ||||||||||
|
|
||||||||||
| > **Summary:** Use only `filesDir` and `noBackupFilesDir` for sensitive data, always encrypting it using platform cryptography or a well-maintained library. | ||||||||||
|
|
||||||||||
| --- | ||||||||||
|
|
||||||||||
| ## Resources | ||||||||||
|
|
||||||||||
| - [`EncryptedFile` (Android API Reference)](https://developer.android.com/reference/androidx/security/crypto/EncryptedFile) | ||||||||||
| - [`EncryptedSharedPreferences` (Android API Reference)](https://developer.android.com/reference/androidx/security/crypto/EncryptedSharedPreferences) | ||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The DEMO must follow the content guidelines (https://mas.owasp.org/contributing/writing-content/mastg-demo.instructions). For example, the "Evaluation" section should explain each relevant line of the output that is used as an evidence to give a FAIL to the test. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| package org.owasp.mastestapp | ||
|
|
||
| import android.content.Context | ||
| import android.database.sqlite.SQLiteDatabase | ||
| import android.util.Log | ||
|
|
||
| class MastgTest (private val context: Context){ | ||
|
|
||
|
|
||
| fun writeSensitiveDataToUnencryptedSQLite() { | ||
| val username = "demoUser_sqlite" | ||
| val userEmail = "john.doe@maswe-0006.com" | ||
| val accessToken = "ghp_1234567890abcdefghijklmnopqrstuvABCD" | ||
| val dbName = "PrivateUnencryptedData" | ||
|
|
||
| try { | ||
| val db: SQLiteDatabase = context.openOrCreateDatabase( | ||
| dbName, | ||
| Context.MODE_PRIVATE, | ||
| null | ||
| ) | ||
|
|
||
| db.execSQL("CREATE TABLE IF NOT EXISTS Users(Username VARCHAR, Email VARCHAR, Token VARCHAR);") | ||
|
|
||
|
|
||
| db.execSQL("DELETE FROM Users;") | ||
|
|
||
| db.execSQL("INSERT INTO Users VALUES('$username', '$userEmail', '$accessToken');") | ||
|
|
||
| db.close() | ||
| Log.i("MASTG-SQLITE", "Data written to unencrypted database: $dbName. Ready for ADB extraction.") | ||
|
|
||
| } catch (e: Exception) { | ||
| Log.e("MASTG-SQLITE", "Error writing data to SQLite: ${e.message}") | ||
| } | ||
| } | ||
|
|
||
| fun mastgTest(): String { | ||
| writeSensitiveDataToUnencryptedSQLite() | ||
| return "SQLite Demo Complete. Database 'PrivateUnencryptedData' created with PII and Token." | ||
| } | ||
| } |
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This Java file does not seem to be the Java file resulting from decompiling the DEMO app. As commented previously, follow the DEMO guidelines (https://mas.owasp.org/contributing/writing-content/mastg-demo.instructions/) to provide these files accordingly. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| package org.owasp.mastestapp; | ||
|
|
||
| import android.content.Context; | ||
| import android.util.Log; | ||
| import kotlin.Metadata; | ||
| import kotlin.jvm.internal.Intrinsics; | ||
|
|
||
| /* compiled from: MastgTest.kt */ | ||
| @Metadata(d1 = {"\u0000\u0018\n\u0002\u0018\u0002\n\u0002\u0010\u0000\n\u0000\n\u0002\u0018\u0002\n\u0002\b\u0003\n\u0002\u0010\u000e\n\u0000\b\u0007\u0018\u00002\u00020\u0001B\u000f\u0012\u0006\u0010\u0002\u001a\u00020\u0003¢\u0006\u0004\b\u0004\u0010\u0005J\u0006\u0010\u0006\u001a\u00020\u0007R\u000e\u0010\u0002\u001a\u00020\u0003X\u0082\u0004¢\u0006\u0002\n\u0000¨\u0006\b"}, d2 = {"Lorg/owasp/mastestapp/MastgTest;", "", "context", "Landroid/content/Context;", "<init>", "(Landroid/content/Context;)V", "mastgTest", "", "app_debug"}, k = 1, mv = {2, 0, 0}, xi = 48) | ||
| /* loaded from: classes3.dex */ | ||
| public final class MastgTest { | ||
| public static final int $stable = 8; | ||
| private final Context context; | ||
|
|
||
| public MastgTest(Context context) { | ||
| Intrinsics.checkNotNullParameter(context, "context"); | ||
| this.context = context; | ||
| } | ||
|
|
||
| public final String mastgTest() throws Exception { | ||
| DemoResults r = new DemoResults("0000"); | ||
| try { | ||
| Log.d("MASTG-TEST", "Hello from the OWASP MASTG Test app."); | ||
| r.add(Status.PASS, "The app implemented a demo which passed the test with the following value: 'Hello from the OWASP MASTG Test app.'"); | ||
| r.add(Status.FAIL, "The app implemented a demo which failed the test."); | ||
| throw new Exception("Example exception: Method not implemented."); | ||
| } catch (Exception e) { | ||
| r.add(Status.ERROR, e.toString()); | ||
| return r.toJson(); | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| --- Users Table Content --- | ||
| demoUser_sqlite|john.doe@maswe-0006.com|MASTG_ACCESS_TOKEN_B4C8D2F0E7A |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| #!/bin/bash | ||
|
macik09 marked this conversation as resolved.
|
||
|
|
||
| PACKAGE="org.owasp.mastestapp" | ||
| DB_NAME="PrivateUnencryptedData" | ||
| DB_PATH="/data/data/$PACKAGE/databases/$DB_NAME" | ||
| SDCARD_PULL_PATH="/sdcard/$DB_NAME.db" | ||
| OUTPUT_DB_FILE="${DB_NAME}.db" | ||
| OUTPUT_TXT_FILE="output.txt" | ||
|
|
||
| # Copy Database to the sdcard | ||
| if ! adb shell "run-as $PACKAGE cp databases/$DB_NAME $SDCARD_PULL_PATH"; then | ||
| echo "run-as failed, trying su" | ||
| adb shell "su 0 sh -c 'cp $DB_PATH $SDCARD_PULL_PATH'" | ||
| fi | ||
|
|
||
| # Pull the file to the host machine | ||
| adb pull "$SDCARD_PULL_PATH" . | ||
|
|
||
| echo "--- Users Table Content ---" > $OUTPUT_TXT_FILE | ||
| sqlite3 "$OUTPUT_DB_FILE" "SELECT * FROM Users;" >> $OUTPUT_TXT_FILE | ||
|
|
||
| adb shell "rm $SDCARD_PULL_PATH" 2>/dev/null | ||
| rm $OUTPUT_DB_FILE 2>/dev/null | ||
|
macik09 marked this conversation as resolved.
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,11 +1,37 @@ | ||||||
| --- | ||||||
| title: Sensitive Data Stored Unencrypted via SQLite | ||||||
| platform: android | ||||||
| title: Static Analysis for Unencrypted Sensitive Data in SQLite | ||||||
| id: MASTG-TEST-0304 | ||||||
| type: [static, dynamic] | ||||||
| type: [static] | ||||||
| weakness: MASWE-0006 | ||||||
| best-practices: [] | ||||||
| profiles: [L1, L2] | ||||||
| status: placeholder | ||||||
| note: This test checks if the app uses the default SQLite API (e.g., `SQLiteOpenHelper`, `context.openOrCreateDatabase`) to store sensitive data (e.g., tokens, PII) in an unencrypted database file within the app's sandbox. It confirms the absence of secure alternatives like SQLCipher or encrypted databases. | ||||||
| best-practices: [MASTG-BEST-0025] | ||||||
| profiles: [L2] | ||||||
| status: new | ||||||
| --- | ||||||
|
|
||||||
| ## Overview | ||||||
|
|
||||||
| This test verifies whether the app's code uses SQLite APIs to store sensitive data — such as tokens, credentials, or PII — without encryption. By default, SQLite databases created using `SQLiteOpenHelper` or `context.openOrCreateDatabase` are not encrypted. | ||||||
|
|
||||||
| ## Steps | ||||||
|
|
||||||
| 1. Obtain the application package (APK) using @MASTG-TECH-0003. | ||||||
|
|
||||||
| 2. Use static analysis (@MASTG-TECH-0014) to review code for calls to SQLite APIs, including: | ||||||
| - `SQLiteOpenHelper` | ||||||
| - `context.openOrCreateDatabase` | ||||||
| - `SQLiteDatabase#insert`, `query`, `execSQL`, etc. | ||||||
|
|
||||||
| 3. Check database creation and insertion logic to determine if: | ||||||
| - the database is created using default SQLite, | ||||||
| - no encryption mechanism (SQLCipher or equivalent) is applied, | ||||||
| - sensitive fields (tokens, secrets, PII) are inserted in plaintext. | ||||||
|
|
||||||
| ## Observation | ||||||
|
|
||||||
| - Identify SQLite databases created in the code. | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Observation must not be "steps" like "identify" or "determine". Observation must declare the output you get after executing all steps and serves as evidence.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||||||
| - Determine whether sensitive data is inserted without encryption. | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
|
||||||
| ## Evaluation | ||||||
|
|
||||||
| The test fails if the app references SQLite APIs and stores sensitive data without any encryption. | ||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,35 @@ | ||||||
| --- | ||||||
| platform: android | ||||||
| title: Runtime Verification of Sensitive Data Stored Unencrypted in SQLite | ||||||
| id: MASTG-TEST-0307 | ||||||
| type: [dynamic, filesystem] | ||||||
| weakness: MASWE-0006 | ||||||
| best-practices: [MASTG-BEST-0025] | ||||||
| profiles: [L2] | ||||||
| status: new | ||||||
| --- | ||||||
|
|
||||||
| ## Overview | ||||||
|
|
||||||
| This test checks at runtime whether sensitive data — tokens, credentials, or PII — is stored in SQLite databases in plaintext. The goal is to verify that data stored in the app's private storage is not left unencrypted. | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are several comments provided by @cpholguera that are marked as resolved but they are not applied to the last commit. This is one example. Maybe you resolved them, but after some changes they are overriden. Please check each of the previously provided suggestions and verify that they are applied here (if applicable). |
||||||
|
|
||||||
| ## Steps | ||||||
|
|
||||||
| 1. Install and run the app on a rooted or emulated device (@MASTG-TECH-0005). | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Starting a device is not anymore needed for the tests. |
||||||
|
|
||||||
| 2. Trigger app functionality that processes or stores sensitive data. | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
|
||||||
| 3. Access the app's private storage to locate SQLite database files (e.g., `.db`, `.sqlite`, `.sqlite3`) (@MASTG-TECH-0008). | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
|
||||||
| 4. Extract the database files to the host machine using @MASTG-TECH-0003. | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. MASTG-TECH-0003 is about dumping the APK, but not their associated storage files. |
||||||
|
|
||||||
| 5. Inspect the database content using dynamic analysis techniques (@MASTG-TECH-0015) to determine if sensitive data is stored in plaintext. | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is not a 1:1 TECH for this step, it's better to just enforce that the databases must be inspected. |
||||||
|
|
||||||
| ## Observation | ||||||
|
|
||||||
| - Which SQLite databases exist on the device. | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| - Whether sensitive data (tokens, secrets, PII) is present in plaintext. | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
|
||||||
| ## Evaluation | ||||||
|
|
||||||
| The test fails if sensitive data is stored in SQLite without encryption and can be read in plaintext through static or dynamic analysis. | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MASTG-KNOW-0036 is about enforcing updates. What is the correlation between that KNOW file and this BEST file?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While creating this section the IDs were different. MASTG-KNOW-0036 was about storing data using SharedPreferences. I should have used dummy ID. I will modify my PR soon and apply your suggestions.