Memory safety issue in frida fuzzer breaks Windows CI

[Mrmaxmeier]

Windows CI is sad at the moment. I tried investigating and found a gnarly UB issue in the frida fuzzer:

Due to an unsound API in frida-gum (frida/frida-rust#102), this callback references a dangling stack object (helper):

LibAFL/libafl_frida/src/helper.rs

Line 258 in 37bfead

let transformer = Transformer::from_callback(gum, |basic_block, output| {

Unfortunately this is not trivial to fix, as helper is returned (and thus moved) later as part of the FridaInstrumentationHelper::new API.

It seems like this was noticed previously (#992), but it is still an issue.

[tokatoka]

thanks for the investigation

do you think this is related? i feel it is.
#1053

here i tried to make FridaInstrumentationHelper return Result<>, but it simply doesn't work (which is suprising)

[Mrmaxmeier]

Using a Result<> instead wouldn't fix the underlying issue. The closure passed to Transformer::from_callback needs to move || its context (currently it's not moving but referencing it). In order to do so the helper object would need be refactored into something that's reference-counted because we need to move it both into the closure context and return it from the Helper::new function.

[tokatoka]

Yeah i know. I mean Making it return Result<> simply make the entire frida_fuzzer break, which makes no sense.
So I guess the root cause of my issue is the same as yours.

[domenukk]

Any chance you can open a PR for that against frida @Mrmaxmeier ? I'd be more than happy to get frida fixed at some point :D

[Mrmaxmeier]

So I guess the root cause of my issue is the same as yours.

Yes, wrapping the Helper in a Result probably changed the layout on the stack, which messes up the dangling reference in the callback closure. It still feels a bit like a miracle though that the code has ever worked correctly. 🙃

Any chance you can open a PR for that against frida?

I've opened frida/frida-rust#103, but the PR is not required to fix the issue on LibAFL's side.