Skip to content

[Mach-O] Catch exceptions thrown in universal transform header parsing - #8577

Open
emesare wants to merge 1 commit into
devfrom
test_fix_exceptional_universal_transform_parse_0
Open

emesare wants to merge 1 commit into
devfrom
test_fix_exceptional_universal_transform_parse_0

Conversation

@emesare

@emesare emesare commented Sep 23, 2026 •

Copy link
Copy Markdown
Member

Fixes potential OOB reads (which in the binary reader cause exceptions) caused by malformed data. We might want to instead catch exceptions higher up the call stack, not sure. If we instead just want to guard every read with some bounds check or something im amenable to that, but honestly that might just bloat code up with potential bad guards 🤣 (and we typically only do that when we have a recoverable situation)

Fixes https://github.com/Vector35/binaryninja/issues/1911

Fixes potential OOB reads caused by malformed data. We might want to instead catch exceptions higher up the call stack, not sure.

Fixes Vector35/binaryninja#1911
@emesare
emesare requested a review from bpotchik September 23, 2026 17:26
@emesare emesare self-assigned this Sep 23, 2026
@emesare emesare changed the title [Mach-O] Catch exceptions thrown in universal transform parsing [Mach-O] Catch exceptions thrown in universal transform header parsing Sep 23, 2026

@bpotchik bpotchik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The exception fix prevents the crash, but it looks like there is a root cause here. There is an integer overflow that allows the out of bounds read. The bug is in two places, copied from the original FAT macho view.

  • universaltransform.cpp
    • size_t requiredFatHeaderSize = fatHeader.nfat_arch * (isFat64 ? 32 : 20) + 8;
  • fatmachoview.cpp
    • size_t expectSize = header.nfat_arch * ....

Just add a (size_t) to extend before the multipl on both: fatHeader.nfat_arch and header.nfat_arch

Also, yes I think catching higher up is the right call. Look in api/transform.cpp. Following the same patter in BinaryView::InitCallback(void* ctxt)

You could apply the pattern below to each of DecodeCallback, EncodeCallback, and CanDecodeCallback.

try {
....
catch (const std::exception& e)
{
    LogError("Transform::DecodeWithContext failed: %s", e.what());
    return false;
}
catch (...)
{
    LogError("Transform::DecodeWithContext failed with unknown exception");
    return false;
}

This branch has not been deployed

No deployments
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.

2 participants