Add Tests and Demos about "Sensitive Data Stored Unencrypted via Room"#3541
Add Tests and Demos about "Sensitive Data Stored Unencrypted via Room"#3541macik09 wants to merge 16 commits into
Conversation
cpholguera
changed the title
macik09
force-pushed
the
feature/mastg-roomdb-0306
branch
2 times, most recently
from
e96f24c to
e1f04d1
Compare
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.
| val dbPath = ctx.getDatabasePath("PrivateUnencryptedRoomDB") | ||
| Log.i("MASTG-ROOM", "Database absolute path: ${dbPath.absolutePath}") | ||
|
|
||
| // Wymuś utworzenie katalogu databases jeśli nie istnieje |
There was a problem hiding this comment.
Please, provide all comments / code lines in English :)
| this.context = context; | ||
| } | ||
|
|
||
| public final String mastgTest() throws Exception { |
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.
| @@ -0,0 +1,30 @@ | |||
| #!/bin/bash | |||
There was a problem hiding this comment.
The DEMO executes a run.sh file that extracts the Room database from the app's sandbox, but this is not what is made in the test. Normally, run.sh is a script that automates parts of the test (for example, if the tests consists on hooking several functions, the frida command is attached here for better reproducibility). Please also update run.sh accordingly to the TEST steps when updating the DEMO.
| id: MASTG-TEST-0309 | ||
| type: [dynamic, filesystem] | ||
| weakness: MASWE-0006 | ||
| best-practices: [MASTG-BEST-0025] |
There was a problem hiding this comment.
This BEST is about using PRNGs. How is this related to this test?
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!
Thanks!
|
|
||
| 4. Extract the database files to the host machine using @MASTG-TECH-0003. | ||
|
|
||
| 5. Inspect database contents using a SQLite client or dynamic analysis tool (@MASTG-TECH-0015) to confirm whether sensitive data is stored in plaintext. |
There was a problem hiding this comment.
Such TECH is in TBD, try to use another TECH that is already done and fits better; otherwise, the TECH must be properly documented before relasing this TEST.
| title: Sensitive Data Stored Unencrypted via Android Room DB | ||
| platform: android | ||
| title: Static Analysis for Unencrypted Sensitive Data in Android Room DB | ||
| id: MASTG-TEST-0306 |
There was a problem hiding this comment.
Use a temporal ID to avoid drifts
There was a problem hiding this comment.
While using temporal IDs my build failed because of regex. Is it ok or i misuderstood how to assign fake ID?
There was a problem hiding this comment.
Yes, temporal IDs (e.g. MASTG-TEST-0x01) for the PRs are completely fine and the intended way to do it :)
See https://mas.owasp.org/contributing/writing-content/mastg-test.instructions/#creating-test-ids
|
@jacobocasado The test process is now fully automated via run.sh, providing clear evidence in output.txt which is then mapped in the Evaluation section. I also changed the android_reversed.kt file. From my perspective, looks good now. If you find this approach fine, I will proceed with updating my other pending PRs (SQLite - #3534 and Datastore - #3540 ) in the same manner to ensure consistency. Looking forward to your feedback! |
|
|
||
| ## Overview | ||
|
|
||
| This test verifies whether the app's code uses the [Android Room Persistence Library](https://developer.android.com/training/data-storage/room) to store sensitive data — such as tokens, credentials, or PII — without encryption. By default, Room stores data in unencrypted SQLite databases. |
There was a problem hiding this comment.
@cpholguera Should this be in a specific, little KNOW file talking about Room?
There was a problem hiding this comment.
Also Room is a SQLite wrapper, I don't know if that serves a mention (there is already a KNOW file of SQLite: MASTG-KNOW-0037, https://mas.owasp.org/MASTG/knowledge/android/MASVS-STORAGE/MASTG-KNOW-0037/)
| id: MASTG-TEST-0x01 | ||
| type: [dynamic, filesystem] | ||
| weakness: MASWE-0006 | ||
| best-practices: [MASTG-BEST-0x25] |
There was a problem hiding this comment.
BEST-0x25 is a new BEST file? Maybe you can make a BEST file for Android that recommends Storing Data Encrypted in App Sandbox Directory, like the one that exists for iOS right now (https://mas.owasp.org/MASTG/best-practices/MASTG-BEST-0024/).
You can attach this BEST file here, or in the other PRs, and expand it with examples of each of the ways to store data (Room, SQLite, DataStore) - If you are not doing this now, please create a PR to keep track of the needs of creating a file like this for Android.
There was a problem hiding this comment.
Review done @macik09 , overall, good improvements!
If you have any questions, let me know before proceeding with the next ones!
Co-authored-by: Jaco <43786394+jacobocasado@users.noreply.github.com>
Co-authored-by: Jaco <43786394+jacobocasado@users.noreply.github.com>
macik09
force-pushed
the
feature/mastg-roomdb-0306
branch
from
6ac7b1d to
c06b7ba
Compare
2 participants
This PR closes #3448
##Summary
This PR introduces a new demo and finalizes the corresponding test for the MASVS-STORAGE requirement concerning unencrypted data persistence using the Room Library.
##Description
This submission specifically addresses data persistence via the Android Room Persistence Library by adding MASTG-DEMO-0070 and finalizing MASTG-TEST-0306.
The demo proves that sensitive PII (Email) and a secret (Access Token/Credential) are stored in plaintext within the default SQLite database created by Room. This is due to the lack of integration of an external encryption library (e.g., SQLCipher), confirming the weakness MASWE-0006.
The included run.sh script demonstrates the extraction of the Room database files from the app sandbox, allowing easy inspection of the sensitive contents in plaintext.
[x] I have read the contributing guidelines.