libafl_frida: Make cmplog work on x64

[expend20]

#823

Currently it stuck on not producing expected values to the input, despite call to __libafl_targets_cmplog_instructions seems correct at first glance

[tokatoka]

I tried this on linux. and from what I observed, the recorded metadata is not correct.

For example, I changed the harness (in frida_libpng) to

HARNESS_EXPORTS extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data,
                                                      size_t         size) {
  if (size >= 8 && *(uint32_t *)data == 0x11112222) { abort(); }
  if (size < kPngHeaderSize) { return 0; }

  return 0;
}

but then the recorded metadata is

 meta: Some(CmpValuesMetadata { list: [U64((474e5089, da))] })

[domenukk]

Did you consider https://github.com/iximeow/yaxpeax-x86 ? If we do need more dependencies we might as well go with something that's potentially cross architecture?

[expend20]

I tried this on linux. and from what I observed, the recorded metadata is not correct

could you try again please? @tokatoka

Did you consider https://github.com/iximeow/yaxpeax-x86 ? If we do need more dependencies we might as well go with something that's potentially cross architecture?

Thanks for the suggestion @domenukk 👍 however, I also need to generate instructions with registers taken from disassembly. Frida InstructionWriter lacks support for lower registers and dynasm seems not a good fit for it. iced seems a perfect fit since it can do both, decoding and encoding at the same time. Never the less, I completely understand your request not to inject additional depencies and I'm happy to explore any other options, if any.

[tokatoka]

i'll check this soon

[tokatoka]

but do you have any idea for resolving register collision?
if not i think we can simply handle every corner cases.

because now you hide the register name behind arg1, arg2, ... , so it would be not too hard to enumerate every cases

[expend20]

Register collision fix is already pushed. Do you experience any issues related to this?

…

________________________________ From: tokatoka ***@***.***> Sent: Thursday, December 28, 2023 8:04:57 PM To: AFLplusplus/LibAFL ***@***.***> Cc: expend20 ***@***.***>; Author ***@***.***> Subject: Re: [AFLplusplus/LibAFL] DRAFT: POC attempt to make cmplog work on x64 (PR #1713) but do you have any idea for resolving register collision? if not i think we can simply handle every corner cases. — Reply to this email directly, view it on GitHub<#1713 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AIWZYPYYHJAUOOEULDEGRXLYLYJLTAVCNFSM6AAAAABAG5J2EWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNZRGY2TIOBXGA>. You are receiving this because you authored the thread.Message ID: ***@***.***>

[tokatoka]

no i haven't got a machine to test yet 😅

[domenukk]

This is still a draft, right?

[expend20]

This is still a draft, right?

yeah, need to craft fuzz targets in assembly to cover all the edge cases of instrumentation

[tokatoka]

works for me 👍

[tokatoka]

look good.
ready to merge?

[expend20]

ready to merge?

I'm pretty close to removing the draft mark, just getting some bugs with more testing

[domenukk]

Some tiny clippy nags remain:

error: casting `u64` to `i64` may wrap around the value
   --> libafl_frida/src/cmplog_rt.rs:671:17
    |
671 |                 instruction.memory_displacement64() as i64,
    |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#cast_possible_wrap
note: the lint level is defined here
   --> libafl_frida/src/lib.rs:10:9
    |
10  | #![deny(clippy::pedantic)]
    |         ^^^^^^^^^^^^^^^^
    = note: `#[deny(clippy::cast_possible_wrap)]` implied by `#[deny(clippy::pedantic)]`

error: could not compile `libafl_frida` (lib test) due to 1 previous error

/edit: removed unrelated warnings from this comment.
For the i64, either use try_from, or if speed is an issue here, allow it with #[allow(clippy:...)] (and maybe add a debug_assert)

[expend20]

Some tiny clippy nags remain:

yeah I'll fix that, I also have some tests which are not yet commited but failing on my machine

For the i64, either use try_from, or if speed is an issue here, allow it with #[allow(clippy:...)] (and maybe add a debug_assert)

in that specific case I need to cast unsigned to signed and try_ gives me a panic, so I removed it

PS: bear with me guys, I'll remove Draft mark, squash commits and let you know explicitly when it's ready for final review

[domenukk]

Sorry, just tried to be helpful :)
Just add a clippy allow in this case (and maybe a comment)

[domenukk]

No need to squash btw, we squash everything when we merge anyway

[expend20]

Should be ready for final review now (with the exception of iced dependency). cc @tokatoka @domenukk

[domenukk]

Diff in /home/runner/work/LibAFL/LibAFL/libafl_frida/src/cmplog_rt.rs at line 879:
         Self::new()
     }
 }

This will need a cargo +nightly fmt, otherwise looks good! :)

[expend20]

This will need a cargo +nightly fmt, otherwise looks good! :)

Hm, I didn't write that code, and it's how it is in main branch, I guess during one of the rebases I enabled clippy, but it's actually disabled in main. Leaving as-is for now.

[expend20]

CI is 🟢 and there is no open conversations at the moment ✅

[tokatoka]

thanks @expend20

[domenukk]

Aweseome! :)