Cleanup & a few bug fixes regarding method re-implementation#1
Cleanup & a few bug fixes regarding method re-implementation#1oleavr merged 1 commit intofrida:masterfrom
Conversation
- Removed resolution to wrong symbol `artQuickGenericJniTrampoline` (not a quick code, just inner impl.) - Allow ArtMethod instance mutation via kFastNative flag (needed by method restoration when calling the original method) - Sepearation between allocation ($alloc, corresponds to new-instance or Unsafe.allocateInstance) and initialization ($init, coressponds to <init>) - allows overriding of constructors - Selective patching and restoration of individual ArtMethod fields - Corrected a bug causing stack frame recognition of re-implemented methods to fail due to incorrect resolution of quick_generic_jni_trampoline
oleavr
left a comment
oleavr
left a comment
There was a problem hiding this comment.
Whoah, awesome work! 🙌
(Just one question before we land this.)
| let remaining = 2; | ||
| for (let offset = 0; offset !== 64 && remaining !== 0; offset += 4) { | ||
| const field = setArgV0.add(offset); | ||
| const getArtMethodSpec = resolveStructSpec.bind({ |
There was a problem hiding this comment.
Did you run into a case where the new struct offset detection got the wrong offsets or failed?
There was a problem hiding this comment.
@oleavr Yup. For Android 5.0.x 32-bit the offset calculation of entry_point_from_quick_code_ is wrong as the width of the preceding field is always 64-bit (uint64_t) no matter which CPU architecture the system is on. I've had no problem for 64-bit or other versions though, so I just kept the offset detection as a fallback for future versions. It may be okay to make it a default for every versions except Lollipop, but I'm a bit unsure..
Additionally, support for 64-bit Android version is missing due to insufficient offset spec info of art::Runtime and art::ClassLinker (mandatory for correct resolution of art_quick_generic_jni_stub so that stack walking would correctly recognize it's actually a JNI method when VM is in preloaded OAT image mode). We could probably work around this by injecting a native stub method and using its quickCode as the JNI stub. Maybe I should work on it later...
There was a problem hiding this comment.
Ahh gotcha. That sounds good. I have a 64-bit device running 6.0, so I can have a look at figuring out those offsets. But first, let's merge this. Awesome work!
There was a problem hiding this comment.
@iamahuman FWIW I have now split index.js into multiple modules, and implemented calculation of all the fields:
https://github.com/frida/frida-java/blob/6f93ee08924231dae50565511d1ad1532055eedd/lib/android.js
This test is now passing with four test-devices connected -- three of which are 32-bit ARM emulators running 5.0, 5.1, and 6.0, and the last of which is a 64-bit Samsung Galaxy S6 Edge device:
https://github.com/frida/frida-java/blob/6f93ee08924231dae50565511d1ad1532055eedd/test/android.js#L22-L111
(Yeah the test is a bit verbose, but at least it's very explicit.)
Some testing still remains, but should hopefully be able to release this soon.
|
These improvements have now been released in Frida 8.1.14. Thanks a lot! |
|
Oh, nice work! I didn't thought the class_linker_ field would be
probe-able, hope it'll work well across versions... :D
|
ghost
mentioned this pull request
2 participants
artQuickGenericJniTrampoline(not a quick code, just inner impl.)Suggestions are welcome!