-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Port MASTG test 0019 (by @guardsquare) #3030
Changes from 2 commits
d3cb11b
1f8be5d
355ea51
4127f2c
2898338
8cb1160
b047054
e89ee89
cbf0815
2e0f6d3
707f147
dc19857
5601893
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,25 @@ | ||
--- | ||
title: HTTP URLs | ||
platform: android | ||
id: MASTG-TEST-0x19-1 | ||
type: [static] | ||
weakness: MASWE-0050 | ||
--- | ||
|
||
## Overview | ||
|
||
The app should not contain any HTTP URLs which might be used for communicating with a server. | ||
|
||
## Steps | ||
|
||
1. Reverse engineer the app (@MASTG-TECH-0017). | ||
2. Run a static analysis (@MASTG-TECH-0014) tool and look for any `http://` URLs. | ||
3. Verify the found URLs are actually used for communication. | ||
|
||
## Observation | ||
|
||
The output contains a list of URLs which are used for communication. | ||
cpholguera marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
## Evaluation | ||
|
||
The test case fails if any HTTP URLs are used for communication. | ||
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. explain how we'd know that and refer to techniques (traffic trace, hooking networking APIs, static looking for networking APIs and inspecting the code, etc.). You could have an sentence and then some bullet points for those cases with a short explanation for each of them. 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. See other comments, I think we have a different understanding of this test :) 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. See my new comment above, in the evaluation we can now use the output and explain how to discard false positives. This is not an easy task, but there are some things we can do to get a better list. |
cpholguera marked this conversation as resolved.
Show resolved
Hide resolved
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,25 @@ | ||||||
--- | ||||||
title: SSLSockets without Hostname Verification | ||||||
platform: android | ||||||
id: MASTG-TEST-0x19-2 | ||||||
type: [static] | ||||||
weakness: MASWE-0050 | ||||||
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. Wouldn't https://mas.owasp.org/MASWE/MASVS-NETWORK/MASWE-0052/ be a better fit? |
||||||
--- | ||||||
|
||||||
## Overview | ||||||
|
||||||
`SSLSocket` does not perform hostname verification (see ["Android documentation"](https://developer.android.com/privacy-and-security/security-ssl#WarningsSslSocket)). This needs to be implemented securely by the app itself. | ||||||
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. If we generalize this test to be "SSLSockets not Properly Verifying Hostnames" or similar we can include a reference to https://developer.android.com/privacy-and-security/risks/unsafe-hostname and shortly explain the cases mentioned when even if the app seems to perform hostname verification (because it uses the related classes/methods) it's doing it incorrectly. It'd be good to mention the related classes/methods such as getDefaultHostnameVerifier or HostnameVerifier#verify |
||||||
|
||||||
## Steps | ||||||
|
||||||
1. Reverse engineer the app (@MASTG-TECH-0017). | ||||||
2. Run a static analysis (@MASTG-TECH-0014) tool and look for all usages of `SSLSocket`. | ||||||
cpholguera marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
3. Verify each usage performans manual hostname verification correctly. | ||||||
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. After changing the above, we should update the steps and be more specific. We can tell them to search for uses of Examples (not for linking here, but maybe to use later for demos): |
||||||
|
||||||
## Observation | ||||||
|
||||||
The output contains a list locations where `SSLSocket` is used and if hostname verification is done correctly for each. | ||||||
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 case fails if any hostname verification is missing, or implemented insecurely. | ||||||
cpholguera marked this conversation as resolved.
Show resolved
Hide resolved
|
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,28 @@ | ||||||||||
--- | ||||||||||
title: Cleartext traffic permitted | ||||||||||
cpholguera marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||
platform: android | ||||||||||
id: MASTG-TEST-0x19-3 | ||||||||||
type: [static] | ||||||||||
weakness: MASWE-0050 | ||||||||||
--- | ||||||||||
|
||||||||||
## Overview | ||||||||||
|
||||||||||
Since Android 9 (API level 28) cleartext HTTP traffic is blocked by default (thanks to the [default Network Security Configuration](../../../Document/0x05g-Testing-Network-Communication.md#default-configurations)) but there are multiple ways in which an application can still send it: | ||||||||||
|
||||||||||
- Setting the [`android:usesCleartextTraffic`](https://developer.android.com/guide/topics/manifest/application-element#usesCleartextTraffic "Android documentation - usesCleartextTraffic flag") attribute of the `<application>` tag in the AndroidManifest.xml file. Note that this flag is ignored in case the Network Security Configuration is configured. | ||||||||||
- Configuring the Network Security Configuration to enable cleartext traffic by setting the `cleartextTrafficPermitted` attribute to true on `<domain-config>` elements. | ||||||||||
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
|
||||||||||
|
||||||||||
## Steps | ||||||||||
|
||||||||||
1. Reverse engineer the app (@MASTG-TECH-0017). | ||||||||||
2. Verify `usesCleartextTraffic` is not set to `true` in the AndroidManifest.xml | ||||||||||
3. Inspect the AndroidManifest.xml, and check if a `networkSecurityConfig` is set in the `<application>` tag. If yes, inspect the referenced file, and make sure `cleartextTrafficPermitted` is not set to `true` for any domain. | ||||||||||
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
|
||||||||||
|
||||||||||
## Observation | ||||||||||
|
||||||||||
The output contains a list of domains for which cleartext traffic is enabled. | ||||||||||
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. More accurate like this I think.
Suggested change
|
||||||||||
|
||||||||||
## Evaluation | ||||||||||
|
||||||||||
The test case fails if any cleartext traffic is permitted. | ||||||||||
cpholguera marked this conversation as resolved.
Show resolved
Hide resolved
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,17 @@ | ||||||||||||||||||||||||||||||||||
--- | ||||||||||||||||||||||||||||||||||
title: Cleartext traffic is allowed for cross-platform frameworks | ||||||||||||||||||||||||||||||||||
cpholguera marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||
platform: android | ||||||||||||||||||||||||||||||||||
id: MASTG-TEST-0x19-4 | ||||||||||||||||||||||||||||||||||
type: [static] | ||||||||||||||||||||||||||||||||||
weakness: MASWE-0050 | ||||||||||||||||||||||||||||||||||
--- | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
## Overview | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
Cross-platform frameworks (e.g. Flutter, React native, ...), typically have their own implementations for HTTP libraries, where cleartext traffic can be allowed. | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
## Steps | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
## Observation | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
## Evaluation | ||||||||||||||||||||||||||||||||||
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. We can do this as long as there's a new Issue. Please mention the issue in the PR description.
Suggested change
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
--- | ||
title: Cleartext traffic observed | ||
platform: network | ||
id: MASTG-TEST-0x19-5 | ||
type: [dynamic] | ||
weakness: MASWE-0050 | ||
--- | ||
|
||
## Overview | ||
|
||
Intercept the tested app's incoming and outgoing network traffic and make sure that this traffic is encrypted. | ||
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. Rephrase to highlight more why we're doing this test, what's the value/benefit. For example, this are REAL cleartext connections not just potential ones (as it is the case if we just retrieve the hardcoded URLs). Limitations:
|
||
|
||
## Steps | ||
|
||
You can use one of the following approaches: | ||
cpholguera marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
- Set up @MASTG-TECH-0010 (for Android) or @MASTG-TECH-0062 (for iOS) to capture all traffic and make sure no communication is done in cleartext. | ||
cpholguera marked this conversation as resolved.
Show resolved
Hide resolved
|
||
- Capture all traffic with an interception proxy like @MASTG-TOOL-0077, @MASTG-TOOL-0079, or @MASTG-TOOL-0097 and make sure no request is done in cleartext. Interception proxies like Burp and OWASP ZAP will show HTTP(S) traffic only. You can, however, use a Burp plugin such as [Burp-non-HTTP-Extension](https://github.com/summitt/Burp-Non-HTTP-Extension "Burp-non-HTTP-Extension") or the tool [mitm-relay](https://github.com/jrmdev/mitm_relay "mitm-relay") to decode and visualize communication via XMPP and other protocols. | ||
|
||
Note: Some applications may not function correctly with proxies like Burp and OWASP ZAP because of Certificate Pinning. In such a scenario, you can still use the other technique. | ||
cpholguera marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
## Observation | ||
|
||
The output contains a list of cleartext network requests. | ||
cpholguera marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
## Evaluation | ||
|
||
The test case fails if any cleartext requests are logged. | ||
cpholguera marked this conversation as resolved.
Show resolved
Hide resolved
|
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.
I'd rather explain that an app may have hardcoded HTTP URLs in the app binary, libs binaries and other places within the APK. And note that those URLs may be an indication of insecure connections but not always.
Limitations (draft):
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.
I am not sure about the first bullet. I already added a 3rd step to (likely manually) verify that the URLs are actually used for communication.
Otherwise quite a few HTTP URLs probably will show up.