-
Notifications
You must be signed in to change notification settings - Fork 267
Description
Version and Platform:
- Binary Ninja Version: 5.3.8753-dev
- Edition: Personal
- OS: Ubuntu Linux
- OS Version: 24.04
- CPU Architecture: x64
Bug Description:
The default block analysis operates on uninitialized data in a special case.
An architecture needs to perform an indirect call with branch delay to a no-return function.
This triggers at least one call to arch->GetInstructionInfo on uninitialized stack data as opcode.
Steps To Reproduce:
- Use small reproduction plugin
river image aggregates delicately - Execute
test.pyin python console - See warning in log output
Expected Behavior:
I am not completely sure about the expected behavior because the instrLength that it calculates is never used.
However, block analysis should never pass uninitialized stack data to an architecture plugin.
If this part of the analysis is not needed anymore, removing seems the best solution.
Otherwise, the following should probably be changed aside from using instrLength.
The code could work on the remaining data in opcode or it should initialize delayOpcode.
Decrementing delayInfo.delaySlots-- inside the loop should probably be replaced with info.delaySlots.
Other parts of default ABB seem to ignore branches in delay slots and delayInfo is overwritten in each iteration.
Binary:
Plugin for reproduction: river image aggregates delicately
Additional Information:
Version and Platform specify how I verified this bug.
The issue applies to stable and dev versions since ABB has been open-sourced.
The relevant code did not change in that time.
Relevant part of defaultabb.cpp, from line 520:
size_t instrLength = info.length;
if (info.delaySlots)
{
InstructionInfo delayInfo;
delayInfo.delaySlots = info.delaySlots; // we'll decrement this inside the loop
size_t archMax = location.arch->GetMaxInstructionLength();
uint8_t delayOpcode[BN_MAX_INSTRUCTION_LENGTH];
do
{
delayInfo.delaySlots--;
if (!location.arch->GetInstructionInfo(delayOpcode, location.address + instrLength, archMax - instrLength, delayInfo))
break;
instrLength += delayInfo.length;
} while (delayInfo.delaySlots && (instrLength < archMax));
}