Make harness function take mut ref

[novafacing]

Makes Executor's harness function take a mutable input. This allows harnesses to mutate the input (they don't have to, of course). There are a bunch of reasons this might be desirable, but primarily it allows harnesses that know better about the input format to quickly fix up a corpus (for example, if the harness needs more data, it can extend the input to the length it needs). This can be done with a custom mutator (and still should, for example this won't help much with a target that needs grammar-mutated input), but for simple cases this offers an easy out.

This is ostensibly a breaking API change, although only an internal one. Fuzzers with a harness that take a |buf: &BytesInput| will still work and all the test cases pass with only a modification to the run_target function needed when calling it directly, which I don't think is super common.

Anyway, open for discussion!

Example:

#[cfg(test)]
mod tests {
    use libafl::{
        inputs::{HasBytesVec, HasTargetBytes},
        prelude::{
            current_nanos, havoc_mutations,
            inprocess::{InProcessExecutor, InProcessExecutorMut},
            tuple_list, AsMutSlice, BytesInput, CrashFeedback, ExitKind,
            InMemoryCorpus, MaxMapFeedback, Named, Observer, RandPrintablesGenerator,
            SimpleEventManager, SimpleMonitor, StdMapObserver, StdRand, StdScheduledMutator,
            Truncate, UsesInput,
        },
        schedulers::QueueScheduler,
        stages::StdMutationalStage,
        state::StdState,
        Fuzzer, StdFuzzer,
    };
    use serde::{Deserialize, Serialize};
    use std::ptr::write;

    static mut SIGNALS: [u8; 16] = [0; 16];
    static mut SIGNALS_PTR: *mut u8 = unsafe { SIGNALS.as_mut_ptr() };

    #[derive(Debug, Serialize, Deserialize)]
    struct O {
        name: String,
    }

    impl O {
        pub fn new(name: &str) -> Self {
            Self {
                name: name.to_string(),
            }
        }
    }

    impl Named for O {
        fn name(&self) -> &str {
            &self.name
        }
    }

    impl<S> Observer<S> for O
    where
        S: UsesInput,
    {
        fn pre_exec(
            &mut self,
            _state: &mut S,
            input: &<S as libafl::prelude::UsesInput>::Input,
        ) -> Result<(), libafl::Error> {
            println!("pre_exec: {input:?}");
            Ok(())
        }

        fn post_exec(
            &mut self,
            _state: &mut S,
            input: &<S as UsesInput>::Input,
            _exit_kind: &ExitKind,
        ) -> Result<(), libafl::Error> {
            println!("post_exec: {input:?}");
            Ok(())
        }
    }

    /// Assign a signal to the signals map
    fn signals_set(idx: usize) {
        unsafe { write(SIGNALS_PTR.add(idx), 1) };
    }

    #[test]
    fn test_mutable_bytes() {
        let mut harness = |input: &mut BytesInput| {
            let buf = input.bytes_mut();
            println!("{buf:?}");

            if buf.len() > 3 {
                buf.truncate(3);
            }

            signals_set(0);
            if !buf.is_empty() && buf[0] == b'a' {
                signals_set(1);
                if buf.len() > 1 && buf[1] == b'b' {
                    signals_set(2);
                    if buf.len() > 2 && buf[2] == b'c' {
                        return ExitKind::Crash;
                    }
                }
            }
            ExitKind::Ok
        };

        let observer =
            unsafe { StdMapObserver::from_mut_ptr("signals", SIGNALS_PTR, SIGNALS.len()) };

        let test_observer = O::new("test");

        let mut feedback = MaxMapFeedback::new(&observer);

        let mut objective = CrashFeedback::new();

        let mut state = StdState::new(
            StdRand::with_seed(current_nanos()),
            InMemoryCorpus::new(),
            InMemoryCorpus::new(),
            &mut feedback,
            &mut objective,
        )
        .unwrap();

        let mon = SimpleMonitor::new(|s| println!("{s}"));

        let mut mgr = SimpleEventManager::new(mon);

        let scheduler = QueueScheduler::new();

        let mut fuzzer = StdFuzzer::new(scheduler, feedback, objective);

        let mut executor = InProcessExecutorMut::new(
            &mut harness,
            tuple_list!(observer, test_observer),
            &mut fuzzer,
            &mut state,
            &mut mgr,
        )
        .unwrap();

        let mut generator = RandPrintablesGenerator::new(32);

        state
            .generate_initial_inputs(&mut fuzzer, &mut executor, &mut generator, &mut mgr, 8)
            .expect("Failed to generate the initial corpus");
        let mutator = StdScheduledMutator::new(havoc_mutations());
        let mut stages = tuple_list!(StdMutationalStage::new(mutator));

        fuzzer
            .fuzz_loop_for(&mut stages, &mut executor, &mut state, &mut mgr, 10)
            .expect("Error in the fuzzing loop");
    }
}

Output:

pre_exec: BytesInput { bytes: [225, 225, 108, 255, 255, 225, 97, 65, 76, 225, 225, 253, 253, 75, 75, 75, 75, 75, 75, 75, 75] }
[225, 225, 108, 255, 255, 225, 97, 65, 76, 225, 225, 253, 253, 75, 75, 75, 75, 75, 75, 75, 75]
post_exec: BytesInput { bytes: [225, 225, 108] }

[novafacing]

Tested with a simple InMemoryCorpus that prints every corpus entry that gets added and indeed, it only ever gets 3-byte corpus entries.

[domenukk]

Just for reference, this is also needed for #1009

[domenukk]

What should happen if an executor mutates the input during a re-run? (i.e. calibration stage) - store the updated version of the testcase or not?

[novafacing]

Hmm, good question. Since we don't specify an executor for each stage, I think I need to change all the places where run_target is happening in all the existing stages that call it (Generalization, Colorization, Calibration, Tracing), as well as a couple wrapper executors where execution happens twice but the second time makes no sense to allow the testcase to be mutated, otherwise there would be some pretty unexpected behavior. I do like the idea of stages being allowed to be implemented in a way such that testcases can be mutated on re-run, but it's a horrible idea to change the existing behavior :P

[tokatoka]

for calibration stage it's ok

because this

            let input = state.corpus().cloned_input_for_id(corpus_idx)?;

it already clones at every run.

but overall, yeah, I think it's better to check if the other stages does not further mutate any previously touched inputs

[novafacing]

I also wonder about differential executors -- I think in that case, since they wrap another executor and the user chooses it, we can safely say "Use two non-mut versions of the executor and you're good" (e.g. InProcessExecutor not InProcessExecutorMut)?

[novafacing]

Sorry for the bumper cars programming I'm still trying to figure out how to run act on my macbook with podman to run the build steps locally 😭

[domenukk]

What do the Mut executors do what the others don't / why do we need two executors?

[novafacing]

The Mut versions take an FnMut(&mut S::Input) -> ExitKind harness instead of an FnMut(&S::Input) -> ExitKind harness. We need both because it's not possible to create a type that's generic over mutability (for example, we can't just change it to a type parameter F because it's not possible to specify F as a function that can take either a mutable or immutable argument (some people in rust discord sounded somewhat interested in that possibility but it's not possible now/not planned).

[domenukk]

Why not always take a mut then? Are there any drawbacks?

[novafacing]

Basically just breaking existing API -- if we change the harness type the existing executors take, all existing harnesses break :( if that's okay with y'all though, it's much cleaner to do that and just call it a breaking change.

[domenukk]

Let's just make it 0.11.0 and break APIs? We're doing that constantly (hence the 0.)

[tokatoka]

yeah don't worry about breaking apis too much. my other pr about executor changes the executor trait alreadty

[novafacing]

Sounds like a plan, I'll prep that!

[novafacing]

Alrighty I think we're good to go on this one if nobody has a problem with the change!

[tokatoka]

Unhandled exception. System.IO.IOException: No space left on device : '/home/runner/runners/2.305.0/_diag/Worker_20230706-164027-utc.log'
at System.IO.RandomAccess.WriteAtOffset(SafeFileHandle handle, ReadOnlySpan1 buffer, Int64 fileOffset) at System.IO.Strategies.BufferedFileStreamStrategy.FlushWrite() at System.IO.StreamWriter.Flush(Boolean flushStream, Boolean flushEncoder) at System.Diagnostics.TextWriterTraceListener.Flush() at System.Diagnostics.TraceSource.Flush() at GitHub.Runner.Common.TraceManager.Dispose(Boolean disposing) at GitHub.Runner.Common.TraceManager.Dispose() at GitHub.Runner.Common.HostContext.Dispose(Boolean disposing) at GitHub.Runner.Common.HostContext.Dispose() at GitHub.Runner.Worker.Program.Main(String[] args) System.IO.IOException: No space left on device : '/home/runner/runners/2.305.0/_diag/Worker_20230706-164027-utc.log' at System.IO.RandomAccess.WriteAtOffset(SafeFileHandle handle, ReadOnlySpan1 buffer, Int64 fileOffset)
at System.IO.Strategies.BufferedFileStreamStrategy.FlushWrite()
at System.IO.StreamWriter.Flush(Boolean flushStream, Boolean flushEncoder)
at System.Diagnostics.TextWriterTraceListener.Flush()
at GitHub.Runner.Common.HostTraceListener.WriteHeader(String source, TraceEventType eventType, Int32 id)
at GitHub.Runner.Common.HostTraceListener.TraceEvent(TraceEventCache eventCache, String source, TraceEventType eventType, Int32 id, String message)
at System.Diagnostics.TraceSource.TraceEvent(TraceEventType eventType, Int32 id, String message)
at GitHub.Runner.Worker.Worker.RunAsync(String pipeIn, String pipeOut)
at GitHub.Runner.Worker.Program.MainAsync(IHostContext context, String[] args)
System.IO.IOException: No space left on device : '/home/runner/runners/2.305.0/_diag/Worker_20230706-164027-utc.log'
at System.IO.RandomAccess.WriteAtOffset(SafeFileHandle handle, ReadOnlySpan`1 buffer, Int64 fileOffset)
at System.IO.Strategies.BufferedFileStreamStrategy.FlushWrite()
at System.IO.StreamWriter.Flush(Boolean flushStream, Boolean flushEncoder)
at System.Diagnostics.TextWriterTraceListener.Flush()
at GitHub.Runner.Common.HostTraceListener.WriteHeader(String source, TraceEventType eventType, Int32 id)
at GitHub.Runner.Common.HostTraceListener.TraceEvent(TraceEventCache eventCache, String source, TraceEventType eventType, Int32 id, String message)
at System.Diagnostics.TraceSource.TraceEvent(TraceEventType eventType, Int32 id, String message)
at GitHub.Runner.Common.Tracing.Error(Exception exception)
at GitHub.Runner.Worker.Program.MainAsync(IHostContext context, String[] args)

again 😩

[tokatoka]

please keep the non_mut version of run_target, and add mut version of evaluate_input_with_observers too