Unexpected unused variable when analyzing x86_64 Rust binary

Aperta
#7,667 0 commenti 1 reazione 0 assegnatari Vedi su GitHub

Nessuno ha ancora preso questa issue.

Valutazione

Difficoltà
5/5
Tempo stimato
Più di una settimana
Idoneità per principianti
35/100
Tipo di issue
Bug
Chiarezza
Abbastanza chiara
Stato di attività
Ferma
Stack tecnologico
rust

Direzione di ricerca

Open the attached test1.zip binary and navigate to test1::main::h30c3c123818d97db. Inspect the HLIL/MLIL around 0x415877, especially how the stack aggregate and indirect formatter call are represented. Done means the formatter-function store is no longer incorrectly marked dead and the aggregate use is represented more accurately or handled conservatively.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Descrizione

Version and Platform (required):

  • Binary Ninja Version: 5.2.8574-dev, b85f6a6
  • OS: windows
  • OS Version: 11
  • CPU Architecture: x86_64

Bug Description:
Binja wrongly marks a local variable as dead code :

Image

The instruction at 0x415877 is displayed as dead code when it's actually used.

My assumption is that binja's HLIL/MLIL liveness doesn't see the indirect use through a pointer, so it (wrongly) marks the local var_40_1 as dead

The source code is

fn main() {
    if check() {
        println!("{}", log_headers());
    }
}

Which desugars roughly to

if check() {
    let tmp = log_headers();
    std::io::_print(format_args!("{}", tmp));
}

format_args! builds a core::fmt::Arguments struct, which internally contains an array of ArgumentV1 objects where each ArgumentV1 is basically:

struct ArgumentV1<'a> {
    data: *const ();// pointer to the value
    fmt: fn(*const (), &mut Formatter), // formatting function (Display/Debug/etc)
}

If you look at the disasm:

Image

You can see that var_48 in the HLIL is &String (which we can confirm by looking into test1::log_headers) and var_40_1 is a fmt fn. _print uses rdi (pointing to var_38), which in turn contains that args slice pointing to var_48. Inside _print, it will dereference that slice, read both fields of the ArgumentV1, and call the formatter function pointer (the one stored at 0x415877 in the screenshot)
From Binja's perspective, var_40_1 is never read, only &var_48 is passed onward. And binja doesn't know that the callee will load 16 bytes from that address and use the second qword as a function pointer. Thus, the liveness / dead-store analysis says "var_40_1 is assigned but never used → dead store"

This is basically an analysis limitation (arguably a bug?) in how Binja handles aggregates/structs and unknown function calls in HLIL/MLIL, especially for Rust's fat formatting stuff

Steps To Reproduce:
Open the binary, go to test1::main::h30c3c123818d97db

Expected Behavior:
Instruction at 0x00415877 shouldn't appear as dead code, and should in a way or another display that it's part of a struct. IDA displays the following:

    _BYTE v1[24]; // [rsp+8h] [rbp-60h] BYREF
    _QWORD v2[2]; // [rsp+20h] [rbp-48h] BYREF
    _QWORD v3[7]; // [rsp+30h] [rbp-38h] BYREF
    // ... snipped for clarity ...
    test1::log_headers(v1);
    v2[0] = v1;
    v2[1] = <alloc::string::String as core::fmt::Display>::fmt;
    v3[0] = &unk_56CC8;
    v3[1] = 2LL;
    v3[4] = 0LL;
    v3[2] = v2;
    v3[3] = 1LL;
    std::io::stdio::_print(v3);
    <alloc::vec::Vec<T,A> as core::ops::drop::Drop>::drop(v1);
    return <alloc::raw_vec::RawVec<T,A> as core::ops::drop::Drop>::drop(v1);

which looks like a more proper way to display it?

It would be great if Binja could do some amount of stack aggregate recovery here, or at least be more conservative about marking stores as dead when:

  • a contiguous range of stack slots is initialized together, and
  • the address of the first slot is taken (lea reg, [rsp+K]) and passed to a call

Binary:
Attached: test1.zip

Source is :

#![deny(unsafe_op_in_unsafe_fn)]

use std::fmt::Write as _;

const REQ_LINE: &str = "GET /health HTTP/1.1\r\n";
const HDR_HOST_LINE: &str = "Host: api.service.local\r\n";
const HDR_UA_LINE: &str = "User-Agent: audit/1.0\r\n";
const HDR_ACC_LINE: &str = "Accept: application/json\r\n";

pub static POOL: &str = "GET /health HTTP/1.1\r\n\
     Host: api.service.local\r\n\
     User-Agent: audit/1.0\r\n\
     Accept: application/json\r\n";

const REQ_START: usize = 0;
const REQ_END: usize = REQ_START + REQ_LINE.len();

const HOST_START: usize = REQ_END;
const HOST_END: usize = HOST_START + HDR_HOST_LINE.len();

const UA_START: usize = HOST_END;
const UA_END: usize = UA_START + HDR_UA_LINE.len();

const ACC_START: usize = UA_END;
const ACC_END: usize = ACC_START + HDR_ACC_LINE.len();

#[inline(always)]
pub fn S_REQ_LINE() -> &'static str {
    &POOL[REQ_START..REQ_END]
}

#[inline(always)]
pub fn S_HOST() -> &'static str {
    &POOL[HOST_START..HOST_END]
}

#[inline(always)]
pub fn S_UA() -> &'static str {
    &POOL[UA_START..UA_END]
}

#[inline(always)]
pub fn S_ACC() -> &'static str {
    &POOL[ACC_START..ACC_END]
}

#[inline(never)]
fn log_headers() -> String {
    let mut out = String::new();
    let _ = write!(&mut out, "{}{}{}", S_HOST(), S_UA(), S_ACC());
    out
}

#[inline(never)]
fn check() -> bool {
    S_REQ_LINE().starts_with("GET ")
        && S_ACC().ends_with("\r\n")
        && matches!(S_HOST(), "Host: api.service.local\r\n")
}

fn main() {
    if check() {
        println!("{}", log_headers());
    }
}
Lingua principale
C++
Stelle
1.3k
Fork
298
Merge medio
5g 5h
PR unite (30g)
19

Guida per i contributori

Nessuna guida per i contributori indicizzata per questo repository

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di Vector35/binaryninja-api

Tutte le issue di Vector35/binaryninja-api

Issue simili

Altre issue su C++

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.