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

[SharedCache] Fix handling of relative selectors in macOS shared caches #6192

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from

Conversation

bdash
Copy link
Contributor

@bdash bdash commented Nov 25, 2024

Find the relative selector base address in the Objective-C optimization data pointed to by the shared cache header, rather than via __objc_scoffs. That section is only present on iOS, and not for every iOS version that encodes selectors via direct offsets.

This also includes some related improvements:

  1. Direct selectors get their own pointer type so they're rendered correctly in the view.
  2. Method lists encoded as lists of lists are now handled.
  3. The dyld_cache_header type added to the view is truncated to the length of the header in the loaded cache. This ensures it is correctly applied to the view.
  4. A couple of methods that process method IMPs and selectors are updated to check whether the address is valid before attempting to process them. They would otherwise fail by throwing an exception if they proceed. Checking for validity is quicker and makes exception breakpoints usable for investigating incorrect behavior.

Additionally, SharedCache now tracks whether non-image regions are data vs code. This means it can avoid marking some regions as containing code when they don't, reducing the amount of analysis work that has to be done.

@bdash
Copy link
Contributor Author

bdash commented Nov 25, 2024

This requires the fixes from #6172. Without those fixes a lot of the class / method data hasn't been rebased appropriately and so is impossible to correctly interpret.

{
dyld_cache_mapping_info mapping;
auto s2 = s.GetArray();
mapping.address = s2[0].GetUint();

Choose a reason for hiding this comment

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

This line and the 2 below should be using GetUint64

@bdash bdash force-pushed the dsc-relative-selectors-mac branch from 726d6fb to 54e33b2 Compare November 30, 2024 05:42
@plafosse plafosse added this to the Gallifrey milestone Dec 4, 2024
@plafosse plafosse added the File Format: SharedCache Issue with the dyld_shared_cache plugin label Dec 10, 2024
@0cyn
Copy link
Member

0cyn commented Dec 10, 2024

thank you for the PR, I'll be looking at getting this merged today.

@0cyn
Copy link
Member

0cyn commented Dec 10, 2024

This is currently being stalled by an issue happening "at some point" during de-serialization that results in an uncaught exception when re-opening a bndb.

The relevant part of traceback is here:

Terminating with uncaught exception
Here's a stack trace (hopefully):
0   libc++abi.dylib                     0x000000018d61fa9c _ZSt11__terminatePFvvE + 16
1   libc++abi.dylib                     0x000000018d622a48 __cxa_get_exception_ptr + 0
2   libc++abi.dylib                     0x000000018d6229f4 _ZN10__cxxabiv1L22exception_cleanup_funcE19_Unwind_Reason_CodeP17_Unwind_Exception + 0
3   libsharedcache.dylib                0x000000010bd18b48 _ZN2VM15AddressIsMappedEy + 0
4   libsharedcache.dylib                0x000000010bd08a74 BNDSCViewGetAllImages + 388
5   libsharedcacheui.dylib              0x000000011d9c27fc _ZN14SharedCacheAPI11SharedCache9GetImagesEv + 48
6   libsharedcacheui.dylib              0x000000011d9da658 _ZN9QtPrivate15QCallableObjectIZN13DSCTriageViewC1EP7QWidgetN11BinaryNinja3RefINS4_10BinaryViewEEEE4$_15NS_4ListIJEEEvE4implEiPNS_15QSlotObjectBaseEP7QObjectPPvPb + 144
7   QtCore                              0x00000001052de008 _Z10doActivateILb0EEvP7QObjectiPPv + 1332

It is not immediately clear to me what could trigger this issue and there doesn't appear to be a codepath between BNDSCViewGetAllImages and _ZN2VM15AddressIsMappedEy at a brief glance.

This could be the result of an improperly resolved merge conflict on my end, and I'll be able to investigate it more deeply tomorrow.

@bdash
Copy link
Contributor Author

bdash commented Dec 11, 2024

The backtrace is probably misleading because of the failed exception throw. I'd guess VM::MappingAtAddress is throwing MappingReadException. It's immediately prior to AddressIsMapped in the source.

I think this can happen with the problem mentioned at #6192 (comment). If the 64-bit fields of dyld_cache_mapping_info are (correctly) serialized as 64-bit integers but (incorrectly) deserialized as 32-bit integers, they'll be truncated on load and result in bogus addresses.

@bdash bdash force-pushed the dsc-relative-selectors-mac branch from 54e33b2 to bf639fe Compare December 11, 2024 05:25
@bdash
Copy link
Contributor Author

bdash commented Dec 11, 2024

I rebased this PR onto dev. I've confirmed it analyzes a shared cache and can save and then re-open the analysis database.

bdash and others added 3 commits December 12, 2024 10:16
`BackingCache` now tracks the `dyld_cache_mapping_info` for its mappings
so it has access to the memory protections for the region. This means it
can avoid marking some regions as containing code when they don't,
reducing the amount of analysis work that has to be done.

Using `dyld_cache_mapping_info` also makes references to mappings easier
to understand due to its named fields vs the nested `std::pair`s that
were previously in use.
Find the relative selector base address in the Objective-C optimization
data pointed to by the shared cache header, rather than via
`__objc_scoffs`. This is only present on iOS, and not for every iOS
version that encodes selectors via direct offsets.

This also includes some related improvements:
1. Direct selectors get their own pointer type so they're rendered
   correctly in the view.
2. Method lists encoded as lists of lists are now handled.
3. The `dyld_cache_header` type added to the view is truncated to the
   length in the loaded cache. This ensures it is applied to the view.
4. A couple of methods that process method IMPs and selectors are
   updated to check whether the address is valid before attempting to
   process them. They would otherwise fail by throwing an exception if
   they proceed, but checking for validity is quicker and makes
   exception breakpoints usable.
@0cyn 0cyn force-pushed the dsc-relative-selectors-mac branch from bf639fe to c7baf92 Compare December 12, 2024 15:53
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ bdash
❌ 0cyn
You have signed the CLA already but the status is still pending? Let us recheck it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
File Format: SharedCache Issue with the dyld_shared_cache plugin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants