Skip to content
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 0039 (by @guardsquare) #3042

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

titze
Copy link
Collaborator

@titze titze commented Nov 6, 2024

Thank you for submitting a Pull Request to the OWASP MASTG. Please make sure that:

  • Your contribution is written in the 2nd person (e.g. you)
  • Your contribution is written in an active present form for as much as possible.
  • You have made sure that the reference section is up to date (e.g. please add sources you have used, make sure that the references to MITRE/MASVS/etc. are up to date)
  • Your contribution has proper formatted markdown and/or code
  • Any references to website have been formatted as [TEXT](URL “NAME”)
  • You verified/tested the effectiveness of your contribution (e.g.: is the code really an effective remediation? Please verify it works!)

If your PR is related to an issue. Please end your PR test with the following line:
This PR closes #3014.

I did not copy the dynamic part, since this has no benefits over the static analysis (imo).


## Steps

1. View the AndroidManifest.xml, e.g., via @MASTG-TOOL-0121.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TOOL-0121 is objdump (ios)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should use MASTG-TOOL-0123

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But even better, use a technique "Obtaining Information from the AndroidManifest" (or similar title) with 2 sections to

  • get the whole android manifest: can use apktool or jadx/jadx-gui
  • or read properties from it: can use aapt

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Always better to use a tech. from a test rather than a tool.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TheDauntless I think this overlapped with landing another tool. Fixed.

@cpholguera I split it up into tech and tool.


To enable debugging of Webviews, the API `WebView.setWebContentsDebuggingEnabled(true);` can be used to enable WebView debugging (see ["see Android documentation"](https://developer.chrome.com/docs/devtools/remote-debugging/webviews/#configure_webviews_for_debugging)).

Calling this API will enable WebView debugging for **all** of the application's WebViews, independently of the AndroidManifest.xml debuggable flag (See @MASTG-TEST-0x39-1). This allows an attached PC to eavesdrop on, and modify all communication inside WebViews.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this really apply to all webviews even if it's only called in one instance?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes 🤯

This setting applies to all of the application's WebViews.

https://developer.chrome.com/docs/devtools/remote-debugging/webviews/#configure_webviews_for_debugging

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh wow 😨

Calling this API will enable WebView debugging for **all** of the application's WebViews, independently of the AndroidManifest.xml debuggable flag (See @MASTG-TEST-0x39-1). This allows an attached PC to eavesdrop on, and modify all communication inside WebViews.

!!! note Other ways to debug the application
Disabling WebView debugging does not fully prevent all possibilities to debug the app and the WebViews. See @MASWE-0101 for more details on how to prevent debugging.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if this note will confuse more than help. Let me know what you think:

What are other ways of debugging the webview if it's not using this? Do you mean that the app can be debugged by other means (but not necessarily the webviews). I guess an attacker able to use frida could call that to enable debugging ...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant that setting debuggable=false still allows someone with root to attach a debugger or attach Frida, essentially debugging the app. This is handled in MASWE-101, not 100% it needs a mention here.

Comment on lines +21 to +23
2. Inspect all locations where `WebView.setWebContentsDebuggingEnabled` is called, and check
1. if it is set to true, and
2. if the API call can be executed during runtime.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
2. Inspect all locations where `WebView.setWebContentsDebuggingEnabled` is called, and check
1. if it is set to true, and
2. if the API call can be executed during runtime.
2. Obtain all locations where `WebView.setWebContentsDebuggingEnabled` is called and set to `true`.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's meant by "if the API call can be executed during runtime"?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://developer.chrome.com/docs/devtools/remote-debugging/webviews/#configure_webviews_for_debugging contains a code snippet that ties the API call to the manifest debuggable flag.

What this sentence should mean is that the API call itself is not a problem if it is essentially in dead code. Maybe it still should not be in the app...

if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.KITKAT) {
    if (0 != (getApplicationInfo().flags & ApplicationInfo.FLAG_DEBUGGABLE))
    { WebView.setWebContentsDebuggingEnabled(true); }
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, got it! Let's see if we should reformulate and/or use this for the evaluation and remediation as well but with more detail mentioning those conditions.

Comment on lines +27 to +31
The output should contain all locations where `WebView.setWebContentsDebuggingEnabled` can be called with `true` at runtime.

## Evaluation

The test case fails if any locations in the application can be executed ar runtime, where WebView debugging is enabled.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same question as above about the "runtime" aspect. Shouldn't we just say that if we found this being true that must be fixed?

tests-beta/android/MASVS-RESILIENCE/MASTG-TEST-0x39-1.md Outdated Show resolved Hide resolved
Comment on lines +13 to +14
!!! note Other ways to debug the application
Not enabling debugging in the AndroidManifest.xml does fully prevent all possibilities to debug the app. See @MASWE-0101 for more details on how to prevent debugging.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we know about other ways to debug the app we should shortly note them here.

Do you mean something like the attacker can set this attribute (which would invalidate the signature) and repackage & re-sign and be able to debug the app. I think this is something we could note here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant #3042 (comment) here as well.

The resigning is a good point, not sure we want to open this topic here.


## Steps

1. View the AndroidManifest.xml, e.g., via @MASTG-TOOL-0121.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should use MASTG-TOOL-0123


## Steps

1. View the AndroidManifest.xml, e.g., via @MASTG-TOOL-0121.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But even better, use a technique "Obtaining Information from the AndroidManifest" (or similar title) with 2 sections to

  • get the whole android manifest: can use apktool or jadx/jadx-gui
  • or read properties from it: can use aapt


## Steps

1. View the AndroidManifest.xml, e.g., via @MASTG-TOOL-0121.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Always better to use a tech. from a test rather than a tool.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MASTG v1->v2 MASTG-TEST-0039: Testing whether the App is Debuggable (android)
3 participants