Add Tests and Demos about "Sensitive Data Stored Unencrypted via SQLite"#3534
Add Tests and Demos about "Sensitive Data Stored Unencrypted via SQLite"#3534macik09 wants to merge 11 commits into
Conversation
cpholguera
left a comment
cpholguera
left a comment
There was a problem hiding this comment.
Thank you! Just 2 comments for now. We'll review it more in depth soon
|
Important: please remember to always add the I added it for you at the top of the Description. |
macik09
force-pushed
the
MASWE-0006-DEMOS
branch
2 times, most recently
from
d110532 to
77a2a72
Compare
macik09
changed the title
macik09
force-pushed
the
MASWE-0006-DEMOS
branch
from
77a2a72 to
e5286e7
Compare
macik09
force-pushed
the
MASWE-0006-DEMOS
branch
from
e5286e7 to
6bc899e
Compare
Co-authored-by: Carlos Holguera <perezholguera@gmail.com>
macik09
force-pushed
the
MASWE-0006-DEMOS
branch
5 times, most recently
from
9e8f4fb to
1c402d3
Compare
Co-authored-by: Carlos Holguera <perezholguera@gmail.com>
macik09
force-pushed
the
MASWE-0006-DEMOS
branch
from
1c402d3 to
2e1c845
Compare
| type: [static, dynamic] | ||
| weakness: MASWE-0006 | ||
| best-practices: [] | ||
| best-practices: [MASTG-BEST-0074] |
There was a problem hiding this comment.
Please add a new MASTG-BEST file for this. I think you just need one for all these PRs where you cover the best practices for MASWE-0006 on Android
- Title could be "Encrypt Sensitive Data in Private Storage Locations"
- Sections could include (please elaborate, see other best practice files):
-
Avoid storing sensitive data locally if not required for application functionality to reduce the likelihood and impact of this weakness.
-
Use platform-provided features for encrypting data at rest. For example, on Android, use EncryptedFile or EncryptedSharedPreferences (or equivalent, note that they are actually deprecated).
-
Using envelope encryption with Data Encryption Keys (DEK) and Key Encryption Keys (KEK) or equivalent methods.
- Examples mentioning SQLite, DataStore, etc.
-
Use libs like SQLCipher
-
etc.
-
cpholguera
changed the title
Co-authored-by: Carlos Holguera <perezholguera@gmail.com>
|
Hi @Diolor , @cpholguera – what is the current status of this PR? Is there anything blocking the review process that I can assist with? |
| alias: encrypt-sensitive-data | ||
| id: MASTG-BEST-0025 | ||
| platform: android | ||
| knowledge: [MASTG-KNOW-0036, MASTG-KNOW-0037, MASTG-KNOW-0038, MASTG-KNOW-0041] |
There was a problem hiding this comment.
MASTG-KNOW-0036 is about enforcing updates. What is the correlation between that KNOW file and this BEST file?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
|
|
||
| ## 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. |
There was a problem hiding this comment.
| 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. | |
| This test checks at runtime whether sensitive data — tokens, credentials, or PII — is stored in SQLite databases (@MASTG-KNOW-0037) without encryption. The goal is to verify that data stored in the app's private storage is not left unencrypted. |
There was a problem hiding this comment.
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). |
There was a problem hiding this comment.
| 1. Install and run the app on a rooted or emulated device (@MASTG-TECH-0005). |
There was a problem hiding this comment.
Starting a device is not anymore needed for the tests.
|
|
||
| 1. Install and run the app on a rooted or emulated device (@MASTG-TECH-0005). | ||
|
|
||
| 2. Trigger app functionality that processes or stores sensitive data. |
There was a problem hiding this comment.
| 2. Trigger app functionality that processes or stores sensitive data. | |
| 1. Exercise all the functionalities of the app that process or store sensitive data. |
|
|
||
| 2. Trigger app functionality that processes or stores sensitive data. | ||
|
|
||
| 3. Access the app's private storage to locate SQLite database files (e.g., `.db`, `.sqlite`, `.sqlite3`) (@MASTG-TECH-0008). |
There was a problem hiding this comment.
| 3. Access the app's private storage to locate SQLite database files (e.g., `.db`, `.sqlite`, `.sqlite3`) (@MASTG-TECH-0008). | |
| 2. Access the app's private storage (@MASTG-TECH-0008) to locate SQLite database files (e.g., `.db`, `.sqlite`, `.sqlite3`). |
|
|
||
| 3. Access the app's private storage to locate SQLite database files (e.g., `.db`, `.sqlite`, `.sqlite3`) (@MASTG-TECH-0008). | ||
|
|
||
| 4. Extract the database files to the host machine using @MASTG-TECH-0003. |
There was a problem hiding this comment.
| 4. Extract the database files to the host machine using @MASTG-TECH-0003. | |
| 3. Extract the database files of the app to the host machine (@MASTG-TECH-0002). |
There was a problem hiding this comment.
MASTG-TECH-0003 is about dumping the APK, but not their associated storage files.
|
|
||
| 4. Extract the database files to the host machine using @MASTG-TECH-0003. | ||
|
|
||
| 5. Inspect the database content using dynamic analysis techniques (@MASTG-TECH-0015) to determine if sensitive data is stored in plaintext. |
There was a problem hiding this comment.
| 5. Inspect the database content using dynamic analysis techniques (@MASTG-TECH-0015) to determine if sensitive data is stored in plaintext. | |
| 4. Inspect the database content to determine if sensitive data is stored in plaintext. |
There was a problem hiding this comment.
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. |
There was a problem hiding this comment.
| - Which SQLite databases exist on the device. | |
| - The location of the SQLite database files inside the application's private storage. |
|
|
||
| ## Observation | ||
|
|
||
| - Identify SQLite databases created in the code. |
There was a problem hiding this comment.
Observation must not be "steps" like "identify" or "determine". Observation must declare the output you get after executing all steps and serves as evidence.
There was a problem hiding this comment.
|
|
||
| ## App Private Storage | ||
|
|
||
| The following sandboxed locations (`/data/data/<package>/`) are private but not secure against privileged attackers: |
There was a problem hiding this comment.
| The following sandboxed locations (`/data/data/<package>/`) are private but not secure against privileged attackers: | |
| All app-private storage under `/data/data/<package>/` is private but not secure against privileged attackers. The following are common locations of the private storage and their common usage: |
| - `databases` – SQLite databases. | ||
| - `shared_prefs` – SharedPreferences. | ||
|
|
||
| Because sandbox isolation does not mitigate elevated access, **plaintext sensitive data must not be stored** in these locations. |
There was a problem hiding this comment.
| Because sandbox isolation does not mitigate elevated access, **plaintext sensitive data must not be stored** in these locations. | |
| Under normal Android sandboxing, other apps cannot directly read these files. However, if the app data directory becomes accessible, for example on a rooted or compromised device or through backup extraction, any unencrypted values stored in these locations could be extracted. | |
| Therefore, it's recommended to encrypt the sensitive data, even if it's in the app private storage. |
| ## Observation | ||
|
|
||
| - Which SQLite databases exist on the device. | ||
| - Whether sensitive data (tokens, secrets, PII) is present in plaintext. |
There was a problem hiding this comment.
| - Whether sensitive data (tokens, secrets, PII) is present in plaintext. | |
| - Occurrences inside the SQLite database file where the sensitive data (tokens, secrets, PII) is stored in plaintext. |
jacobocasado
left a comment
jacobocasado
left a comment
There was a problem hiding this comment.
@macik09 Review done!
Please check https://mas.owasp.org/contributing/writing-content/mastg-demo.instructions/ and also https://mas.owasp.org/contributing/writing-content/mastg-test.instructions/ (for the TEST) and ensure that they are aligned with the instructions!
I also reviewed other PR (#3541) so if you find anything applicable there or here, ensure that both PRs are aligned!
Thanks!
| ## Observation | ||
|
|
||
| - Identify SQLite databases created in the code. | ||
| - Determine whether sensitive data is inserted without encryption. |
There was a problem hiding this comment.
| - Determine whether sensitive data is inserted without encryption. | |
| - Whether encryption is being applied in the code to the sensitive data before being stored. |
|
|
||
| ## Evaluation | ||
|
|
||
| The test fails if sensitive data is stored in SQLite without encryption and can be read in plaintext through static or dynamic analysis. |
There was a problem hiding this comment.
| The test fails if sensitive data is stored in SQLite without encryption and can be read in plaintext through static or dynamic analysis. | |
| The test fails if sensitive data is stored in SQLite without encryption and can be read in plaintext. |
3 participants
Closes #3450
This PR introduces a new demo for the MASVS-STORAGE requirement.
Description
This demo adds MASTG-DEMO-0065 (Sensitive Data in Unencrypted SQLite) to cover the test case MASWE-0006.
The demo proves that sensitive PII (Email) and an Access Token are stored in plaintext using the default Android SQLite API (
openOrCreateDatabase). Therun.shscript demonstrates that these sensitive contents are trivially accessible from the app's private sandbox on a privileged device.Files Added:
MASTG-DEMO-0065.md: Documentation for the demo.MastgTest.kt: Code snippet showing the vulnerable storage.run.sh: Script to extract the database and generate proof (output.txt).output.txt, Manifests, reversed Java).[x] I have read the contributing guidelines.