Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: set DEBUG_MODE variable and add Conscrypt bypass #41

Merged
merged 1 commit into from
Jan 17, 2025

Conversation

digiz3d
Copy link
Contributor

@digiz3d digiz3d commented Dec 23, 2024

Issue

This PR #38 broke the tools, at least on my setup: DEBUG_MODE is not defined, causing a reference error.

Replicate the issue

android-unpinner all -l -f ./apks/xxxxx.apk
then
frida -U -l tools/android-unpinner/android_unpinner/scripts/hide-debugger.js -l tools/android-unpinner/android_unpinner/scripts/httptoolkit-unpinner.js MyApp
gives
Screenshot 2024-12-28 at 17 15 22

Explanation

Unlike https://github.com/httptoolkit/frida-interception-and-unpinning, we don't have a config file in this project

Defining the variable fixes it and properly allows to apply patches

Bonus

Also added brought back another common bypass (inspired by objection https://github.com/sensepost/objection/blob/master/agent/src/android/pinning.ts#L244)
edit: turns out it was also deleted in #38 , see https://github.com/mitmproxy/android-unpinner/pull/38/files#diff-523754949d164f3759f5cfd8712b416590e6e9cf60810aa1e7ddf19346428793L147

I can make a dedicated PR for the bonus if you prefer

@injust
Copy link
Contributor

injust commented Jan 17, 2025

The re-added bypass was removed upstream in httptoolkit/frida-interception-and-unpinning@6279e6e.

Note the commit message:

Remove overly lax hook on TrustManagerImpl

This isn't required, because our system certificate injection prepopulates the index used by all trust managers anyway, so they trust our cert regardless. As configured, the previous hook just trusted all certificates, exposing 3rd party MitM risk - better to keep it strict for just our certificate where we can.

(I haven't thought about this, nor am I context loaded -- I only dug up the commit.)

@mhils
Copy link
Member

mhils commented Jan 17, 2025

Thanks, @injust! I've somehow missed this PR when it was posted. I'll merge now anyway to get the error away, but if this is indeed not needed I'm happy to accept a PR that removes the extra hook.

@mhils mhils merged commit f5ced37 into mitmproxy:main Jan 17, 2025
@digiz3d
Copy link
Contributor Author

digiz3d commented Jan 21, 2025

I'll merge now anyway to get the error away

No worries ! Thanks for merging @mhils

The re-added bypass was removed upstream

Oki that makes sense, thanks for the explanation @injust 👌

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants