fuzzbench icon indicating copy to clipboard operation
fuzzbench copied to clipboard

Fix a case where `fuzz_target_path` is mysteriously None

Open DonggeLiu opened this issue 2 years ago • 6 comments

This was observed in this experiment. Here is the error log:

Traceback (most recent call last):
  File "/src/experiment/runner.py", line 454, in experiment_main
    runner.conduct_trial()
  File "/src/experiment/runner.py", line 276, in conduct_trial
    self.set_up_corpus_directories()
  File "/src/experiment/runner.py", line 261, in set_up_corpus_directories
    _unpack_clusterfuzz_seed_corpus(target_binary, input_corpus)
  File "/src/experiment/runner.py", line 132, in _unpack_clusterfuzz_seed_corpus
    seed_corpus_archive_path = get_clusterfuzz_seed_corpus_path(
  File "/src/experiment/runner.py", line 98, in get_clusterfuzz_seed_corpus_path
    fuzz_target_without_extension = os.path.splitext(fuzz_target_path)[0]
  File "/usr/local/lib/python3.10/posixpath.py", line 118, in splitext
    p = os.fspath(p)
TypeError: expected str, bytes or os.PathLike object, not NoneType

I could not reproduce this error locally, but this PR should fix the error.

DonggeLiu avatar Aug 21 '23 03:08 DonggeLiu

Not sure this is a good idea. I'm more interested in why this fails. I think this fix could potentially mess up results. If one fuzzer is messing with paths and we can't use the seed corpus it's better it fails then runs without using the seed corpus when others do use it (skewing the results)

jonathanmetzman avatar Aug 23 '23 19:08 jonathanmetzman

And these other fuzzers do appear to find the seed corpus: https://www.fuzzbench.com/reports/experimental/2023-08-04-sfuzz/index.html

jonathanmetzman avatar Aug 23 '23 19:08 jonathanmetzman

Not sure this is a good idea. I'm more interested in why this fails. I think this fix could potentially mess up results. If one fuzzer is messing with paths and we can't use the seed corpus it's better it fails then runs without using the seed corpus when others do use it (skewing the results)

+1 we should fix the root cause here, rather than papering over issues.

oliverchang avatar Aug 23 '23 22:08 oliverchang

Not sure this is a good idea. I'm more interested in why this fails. I think this fix could potentially mess up results. If one fuzzer is messing with paths and we can't use the seed corpus it's better it fails then runs without using the seed corpus when others do use it (skewing the results)

I agree that it's better to let it run without using the seeds: I thought that's the intention of this PR? Without this PR, the program will crash and won't be able to run.

But finding the root cause is a better idea. I will look further into that.

DonggeLiu avatar Aug 24 '23 02:08 DonggeLiu

Not sure this is a good idea. I'm more interested in why this fails. I think this fix could potentially mess up results. If one fuzzer is messing with paths and we can't use the seed corpus it's better it fails then runs without using the seed corpus when others do use it (skewing the results)

I agree that it's better to let it run without using the seeds: I thought that's the intention of this PR? Without this PR, the program will crash and won't be able to run.

We don't agree :-) I think it's better to produce a noticeable failure than a subtly incorrect failure.

But finding the root cause is a better idea. I will look further into that.

jonathanmetzman avatar Aug 28 '23 17:08 jonathanmetzman

We don't agree :-) I think it's better to produce a noticeable failure than a subtly incorrect failure.

Oh, I see!

DonggeLiu avatar Aug 29 '23 03:08 DonggeLiu