[fuzz] fix h1 end to end fuzz test issue#15318
Conversation
Signed-off-by: Asra Ali <asraa@google.com>
|
Just found a very similar issue with h2_capture that I will add for this PR. Will ping when ready for review. |
Signed-off-by: Asra Ali <asraa@google.com>
|
Ready, sorry about that! Would love help for a better solution btw. I cannot actually test this without it being in the OSS-Fuzz environment, I have one concern where this might not work, and that is if the canonical path is not but I can't check that in the logs... all I know is the visible path. I'm not sure if /mnt will be the output of realpath |
adisuissa
left a comment
There was a problem hiding this comment.
Strange bug...
I'm guessing that because the test was able to write the file upon initialization without a failure, and because it only failed on the 33rd iteration (although the previous may have been wrong input) then it might be something external that deletes temporary files, but that's just a guess.
I wonder if using /tmp is the right solution (https://google.github.io/oss-fuzz/further-reading/fuzzer-environment/#file-system).
Seems that the fix for the h1 and h2 fuzzer is inconsistent (h1 uses inlined bytes, and h2 uses illegal path). Should it be that way?
| #ifdef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION | ||
| // Allow temporary files in OSS-Fuzz to prevent failures running (h1/h2)_capture_fuzz_test. | ||
| // Ideally this would be TestEnvironment::temporaryPath() but this avoids depending on test | ||
| // libraries. | ||
| if (absl::StartsWith(canonical_path.rc_, "/mnt")) { | ||
| return true; | ||
| } | ||
| #endif |
There was a problem hiding this comment.
Does this mean that the fuzzer code won't be able to access files in /mnt?
Could this impact fuzz tests that may need to access external files under the /mnt directory?
There was a problem hiding this comment.
OMG thanks for pointing that out. The fix was meant to do the opposite.
The best solution is probably to make
Ideally I want to avoid using the filesystem anyway. |
Signed-off-by: Asra Ali <asraa@google.com>
adisuissa
left a comment
There was a problem hiding this comment.
Thanks for fixing this!
|
/assign @htuch |
Signed-off-by: Asra Ali asraa@google.com
Commit Message: Removes temporary files used for direct responses in h1 and h2 end to end fuzz test.
Additional Description: This was causing a flakey issue where OSS-Fuzz's temporary path used for direct responses looked illegal to Envoy. It seems unlikely that it was one of the deny listed paths in
illegalPathbecause the path is always/mnt. It may be flakey on arealpathfailure when getting the canonical path. Replace with inline bytes since it's better not to mess around with the filesystem anyway.and bootstrap path:
Risk Level: Low
Fixes: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=31617