Skip to content

MSP430: conditional_jump! macro initialises new_true to true, emitting a spurious jump #8516

Description

@Astrael26e4

Version and Platform (required):

  • Binary Ninja Version: 5.0.7648
  • Edition: Non-Commercial
  • OS: Ubuntu
  • OS Version: 24.04
  • CPU Architecture: x64

Bug Description:
On MSP430, a conditional jump whose target is in a different basic block lifts as an LLIL_IF followed by a spurious LLIL_JUMP_TO, rather than LLIL_IF with a resolved goto label.

The target block is then never lifted: none of its instructions appear in the function's LLIL, and HLIL renders the region as while (true) /* nop */ with the code silently missing.

Conditional jumps whose target is inside the current basic block (a self-loop) are unaffected. Unconditional jmp and br are unaffected in both directions.

Steps To Reproduce:
printf '\x3b\x40\x02\x24\x0b\x8f\x7b\x90\x21\x00\x01\x28\x0b\x43\x0f\x4b\x30\x41' > jmp_repro.bin in a terminal or open jmp_repro.bin attached and look at sub_0 (create a function at 0 if sub_0 does not exist).

Expected Behavior:
LLIL_IF with the true branch resolved to a goto label at 0xe, and the block at 0xe lifted.

In pseudo C, the function sub_0 should be

int16_t sub_0(int16_t arg1) {
  int16_t result = 0x2402-arg1;
  
  if (result < 0x21) {
     return result;
  }
  return 0;
}

Screenshots/Video Recording:

Image

Binary:
jmp_repro.zip

Suspected cause (hypothesis, not verified by testing a patched build):
In the conditional_jump! macro in arch/msp430/src/lift.rs, new_true is initialised to true while new_false is initialised to false:

let mut new_true = true;
let mut new_false = false;

The unwrap_or_else closure sets new_true = true, which it already is. So when label_for_address succeeds, new_true remains true and the following block still executes:

if new_true {
    $il.mark_label(&mut true_label);
    $il.jump($il.const_ptr(true_addr)).append();
}

This would append a jump even though if_expr already targeted the resolved label — which matches the observed LLIL_JUMP_TO. The false path, correctly initialised, behaves as expected.

The initialisation is still present in dev as of today.

Suggested fix: let mut new_true = false;

Additional Information:

An ArchitectureHook that omits the new_true block and emits only il.if_expr with the resolved labels produces correct LLIL and HLIL on the reproducer above, and on larger MSP430 binaries.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions