binaryninja-api icon indicating copy to clipboard operation
binaryninja-api copied to clipboard

Default block analysis might operate on unitialized data

Open Alkalem opened this issue 2 months ago • 0 comments

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:

  1. Use small reproduction plugin river image aggregates delicately
  2. Execute test.py in python console
  3. 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.

Screenshots/Video Recording: Image

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));
				}

Alkalem avatar Dec 09 '25 23:12 Alkalem