Skip to content

Commit 94bdcd8

Browse files
committed
Add a setting to opt out of rewriting message sends with visible implementations
The setting is on by default to preserve the current experience. It should likely be revisited as related changes are merged. I find this to be confusing as often as it is helpful since it picks the first implementation of the selector it sees without consideration for the type of the receiver. This is particularly annoying if the binary being analyzed implements a method with the same name as a commonly-used method on a system type (`-description` or `-path`, for instance), or has methods with generic names (`-initWithURL:`) on many types. Explicit cross-references from selectors to method implementations make it possible to see the potential implementations without rewriting the call, and an Objective-C pseudo-language (Vector35/binaryninja-api#6807) provides an even more natural representation of Objective-C message sends without these downsides.
1 parent 0df5ff8 commit 94bdcd8

File tree

2 files changed

+15
-4
lines changed

2 files changed

+15
-4
lines changed

Plugin.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,15 @@ BINARYNINJAPLUGIN bool CorePluginInit()
4141

4242
BinaryNinja::LogRegistry::CreateLogger(PluginLoggerName);
4343

44+
auto settings = BinaryNinja::Settings::Instance();
45+
settings->RegisterSetting("core.function.objectiveC.assumeMessageSendTarget",
46+
R"({
47+
"title" : "Rewrite objc_msgSend calls to first visible implementation",
48+
"type" : "boolean",
49+
"default" : true,
50+
"description" : "Message sends of selectors with any visible implementation are replaced with a direct call to the first visible implementation. Note that this can produce false positives if the selector is implemented by more than one class, or shares a name with a method from a system framework."
51+
})");
52+
4453
return true;
4554
}
4655
}

Workflow.cpp

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,8 @@ bool Workflow::rewriteMethodCall(LLILFunctionRef ssa, size_t insnIndex)
115115
ssa->GetFunction()->SetAutoCallTypeAdjustment(ssa->GetFunction()->GetArchitecture(), insn.address, {funcType, BN_DEFAULT_CONFIDENCE});
116116
// --
117117

118+
if (!BinaryNinja::Settings::Instance()->Get<bool>("core.function.objectiveC.assumeMessageSendTarget"))
119+
return false;
118120

119121
// Check the analysis info for a selector reference corresponding to the
120122
// current selector. It is possible no such selector reference exists, for
@@ -126,6 +128,10 @@ bool Workflow::rewriteMethodCall(LLILFunctionRef ssa, size_t insnIndex)
126128
const auto info = GlobalState::analysisInfo(bv);
127129
if (!info)
128130
return false;
131+
132+
// Attempt to look up the implementation for the given selector, first by
133+
// using the raw selector, then by the address of the selector reference. If
134+
// the lookup fails in both cases, abort.
129135
std::vector<uint64_t> imps;
130136
if (const auto& it = info->selRefToImp.find(rawSelector); it != info->selRefToImp.end())
131137
imps = it->second;
@@ -135,10 +141,6 @@ bool Workflow::rewriteMethodCall(LLILFunctionRef ssa, size_t insnIndex)
135141
if (imps.empty())
136142
return false;
137143

138-
// Attempt to look up the implementation for the given selector, first by
139-
// using the raw selector, then by the address of the selector reference. If
140-
// the lookup fails in both cases, abort.
141-
142144
// k: This is the same behavior as before, however it is more apparent now by implementation
143145
// that we are effectively just guessing which method this hits. This has _obvious_ drawbacks,
144146
// but until we have more robust typing and objective-c type libraries, fixing this would

0 commit comments

Comments
 (0)