Skip to content

Commit 3e7908f

Browse files
bdash0cyn
authored andcommitted
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 2040c37 commit 3e7908f

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
@@ -147,6 +147,8 @@ bool Workflow::rewriteMethodCall(LLILFunctionRef ssa, size_t insnIndex)
147147
ssa->GetFunction()->SetAutoCallTypeAdjustment(ssa->GetFunction()->GetArchitecture(), insn.address, {funcType, BN_DEFAULT_CONFIDENCE});
148148
// --
149149

150+
if (!BinaryNinja::Settings::Instance()->Get<bool>("core.function.objectiveC.assumeMessageSendTarget"))
151+
return false;
150152

151153
// Check the analysis info for a selector reference corresponding to the
152154
// current selector. It is possible no such selector reference exists, for
@@ -158,6 +160,10 @@ bool Workflow::rewriteMethodCall(LLILFunctionRef ssa, size_t insnIndex)
158160
const auto info = GlobalState::analysisInfo(bv);
159161
if (!info)
160162
return false;
163+
164+
// Attempt to look up the implementation for the given selector, first by
165+
// using the raw selector, then by the address of the selector reference. If
166+
// the lookup fails in both cases, abort.
161167
std::vector<uint64_t> imps;
162168
if (const auto& it = info->selRefToImp.find(rawSelector); it != info->selRefToImp.end())
163169
imps = it->second;
@@ -167,10 +173,6 @@ bool Workflow::rewriteMethodCall(LLILFunctionRef ssa, size_t insnIndex)
167173
if (imps.empty())
168174
return false;
169175

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

0 commit comments

Comments
 (0)