Port MASTG-TEST-0008: Checking for Sensitive Data Disclosure Through the User Interface (android) #3495
Conversation
|
@cpholguera @sushi2k could you pls assign , i'll do the review for the pull request |
cpholguera
changed the title
There was a problem hiding this comment.
About the title, id and message: this isn't about privacy. We're actually testing from the security realm.
| id: MASTG-TEST-02te | ||
| type: [static, manual] // TODO evaluate | ||
| weakness: MASWE-0053 | ||
| profiles: [P] |
There was a problem hiding this comment.
This is L2, see MASWE-0053
| profiles: [P] | |
| profiles: [L2] |
There was a problem hiding this comment.
#3464 's test that points to WE-54 with similar characteristics to 53 tests privacy. Can you please elaborate?
There was a problem hiding this comment.
That's the same situation
MASWE-0054: Sensitive Data Leaked via Notifications is L2
We have different Tests there as well one for authenticators (L2) and another for PII (P)
Then the weakness will belong to both profiles. Actually weaknesses would inherit the profiles from the underlying tests.
| platform: android | ||
| title: App Exposing Access and Verification Codes in Text Input Fields | ||
| id: MASTG-TEST-02te | ||
| type: [static, manual] // TODO evaluate |
There was a problem hiding this comment.
There is no category "manual"
| type: [static, manual] // TODO evaluate | |
| type: [static] |
There was a problem hiding this comment.
There was a problem hiding this comment.
The test should focus on the static analysis using Semgrep from the layout files. @cpholguera Could you please share your suggestion
There was a problem hiding this comment.
Please propose how to avoid the (manual) step 3.
There was a problem hiding this comment.
Please propose how to avoid the (manual) step 3.
Could you please elaborate on this?
There was a problem hiding this comment.
Step 3 is required to perform the test. It is a manual step, and I could not think of an automatic way to replace it.
If you wish to mark this test as solely static, we would need to find a way to avoid that step, which to my best knowledge, is not possible.
| Proper masking (dots instead of input characters) of these codes is essential to protect user privacy. This can be achieved by using appropriate input types that obscure the characters entered by the user. | ||
|
|
||
| XML view: | ||
|
|
||
| ```xml | ||
| <EditText | ||
| android:inputType="textPassword" | ||
| ... | ||
| /> | ||
| ``` | ||
|
|
||
| Jetpack Compose: | ||
|
|
||
| ```kotlin | ||
| SecureTextField( | ||
| textObfuscationMode = TextObfuscationMode.RevealLastTyped, // or TextObfuscationMode.Hidden | ||
| ... | ||
| ) | ||
| ``` | ||
|
|
||
| > Note: That even if the SecureTextField is used with `textObfuscationMode` set to `RevealLastTyped` or `Hidden`, it can later be changed to `Visible` programmatically. |
There was a problem hiding this comment.
It would be better to incorporate this into the best practices MASTG-BEST-XXX.
There was a problem hiding this comment.
What purpose do you suggest the best practice would serve?
There was a problem hiding this comment.
This serves to standardize remediation advice, improve developer awareness.
Writing MAS Weaknesses & Tests document : https://docs.google.com/document/d/1EMsVdfrDBAu0gmjWAUEs60q-fWaOmDB5oecY9d9pOlg/edit?tab=t.0#heading=h.94f3yhwcmntt
There was a problem hiding this comment.
Thanks for pointing out the guidelines. My question above was to ask for and understand the value of extracting this note into a separate best practice.
The MASTG Best Practices are a collection of specific strategies and practices that can be used to prevent or mitigate security and privacy risks in mobile apps. What is the specific strategy or practice you wish we recommend to readers so that they can mitigate a security or privacy risk? What is the security risk you see here? Awareness is not considered a risk nor a mitigation.
Co-authored-by: Carlos Holguera <[email protected]>
| @@ -0,0 +1,87 @@ | |||
| package org.owasp.mastestapp | |||
There was a problem hiding this comment.
- Please change the demo. Testing should concentrate on black-box scenarios and demonstrate how an attacker might proceed: acquire the APK, decompile the APK, examine res/layout/ for entries, use semgrep to search for hints/ids such as password|pin|secret|pwd, and verify if any password field lacks android:inputType="textPassword".
- Demo for App Notifications: The old test also had a static and dynamic analysis session, which would be great to migrate to this one too and not lose knowledge.
There was a problem hiding this comment.
- Compose is pretty standard nowadays among developers and is gaining more and more traction over XML views. Why should we focus on XMLs?
- Port MASTG-TEST-0005: Determining Whether Sensitive Data Is Shared with Third Parties via Notifications (android) #3464 addresses notifications
Co-authored-by: Carlos Holguera <[email protected]>
3 participants
This PR closes #2981
Description
Port mastg test 0008 from v1 to v2 incl. demo
The rule is on purpose in Kotlin (and not the usual Java) to detect Compose views.
If we truly want to use a rule against Java code, it is possible, but we would need to import more Java reverse-engineered files. Compose classes create a lot of boilerplate and detection would not be 1:1.
e.g. the semgrep rule would need to be looking for
SecureTextFieldKtinstancesTODO:
tests-beta/android/MASVS-PRIVACY/MASTG-TEST-02te.mdline 5