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

build_new_apk: carry extra files from META-INF over #283

Merged
merged 2 commits into from Oct 17, 2019

Conversation

dnet
Copy link
Contributor

@dnet dnet commented Oct 16, 2019

Some features such as Kotlin coroutines use files stored in the META-INF directory. As described in the Apktool documentation

There is no META-INF dir in resulting apk. Is this ok?

Yes. META-INF contains apk signatures. After modifying the apk it is no longer signed. You can use -c / --copy-original to retain these signatures. However, using -c uses the original AndroidManifest.xml file, so changes to it will be lost.

This results in patchapk producing APKs that crash as soon as Kotlin coroutines (or anything else that depend on files stored there) are used:

10-16 11:14:24.138  9824  9824 E AndroidRuntime: FATAL EXCEPTION: main
10-16 11:14:24.138  9824  9824 E AndroidRuntime: Process: com.example, PID: 9824
10-16 11:14:24.138  9824  9824 E AndroidRuntime: java.lang.IllegalStateException: Module with the Main dispatcher is missing. Add dependency providing the Main dispatcher, e.g. 'kotlinx-coroutines-android'
10-16 11:14:24.138  9824  9824 E AndroidRuntime:        at f.a.b.p.n(SourceFile:4)
10-16 11:14:24.138  9824  9824 E AndroidRuntime:        at f.a.b.p.b(SourceFile:1)
10-16 11:14:24.138  9824  9824 E AndroidRuntime:        at f.a.Z.a(SourceFile:11)
10-16 11:14:24.138  9824  9824 E AndroidRuntime:        at f.a.c.a.a(SourceFile:3)
10-16 11:14:24.138  9824  9824 E AndroidRuntime:        at kotlinx.coroutines.CoroutineStart.invoke(SourceFile:10)

This patch examines whether the META-INF directory contains anything else except the standard files used for jarsigner type (old) APK signatures (*.RSA, *.SF and MANIFEST.MF). If so, it uses the built-in ZIP file support of the Python standard library to append such files. For ZIP paths, / is used as a separator regardless of the password, while for everything else, I used os.path.join and os.sep.

Edit: clarified that / is only used for ZIP paths

@leonjza
Copy link
Member

leonjza commented Oct 16, 2019

Thank you for this patch! 🙌 Just one small change, would you mind extracting this to a new function called where you have the logic now?

@dnet
Copy link
Contributor Author

dnet commented Oct 17, 2019

Sure, I had similar thoughts, but I wasn't sure about how to structure it in a way that fits well with the rest of the code.

  1. Should it be a "public" function just like build_new_apk and called from patch_android_apk right after build_new_apk?
  2. Should it be a "public" function just like build_new_apk but called from build_new_apk?
  3. Should it be a "private" function (prefixed with _) and called from build_new_apk?
  4. Any other combination I didn't think of?

@leonjza
Copy link
Member

leonjza commented Oct 17, 2019

I think a private method prefixed with _, called from build_new_apk makes sense given that no-one outside would probably call this (like cli wrappers etc.), and moving it to a function is really just to encapsulate it into a readable chunk.

Thanks again!

@dnet
Copy link
Contributor Author

dnet commented Oct 17, 2019

Done, see 6fad414

@leonjza
Copy link
Member

leonjza commented Oct 17, 2019

Thanks!

@leonjza leonjza merged commit 5cbb328 into sensepost:master Oct 17, 2019
@dnet dnet deleted the meta-inf branch October 17, 2019 13:36
@dnet
Copy link
Contributor Author

dnet commented Nov 5, 2019

Interestingly, @adibfara fixed this in Apktool (where it should be handled anyway) with iBotPeaches/Apktool@154da4c in May 2019 (!), see also kimocoder/Apktool#2 that groups together the 4 commits including the one above. The only problem is that as of November 2019, the last release of Apktool was 2.4.0 in March 2019, thus this functionality is only available to those who compile from source for themselves.

The relevant question is when they publish a new release that includes this functionality, should the solution merged as part of this PR be modified and if so, how.

@leonjza
Copy link
Member

leonjza commented Nov 5, 2019

We could add a version check for apktool?

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

Successfully merging this pull request may close these issues.

None yet

2 participants