Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds small valid .pic file to fuzzer seed corpus; varies req_comp when fuzz-testing #1455

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

NBickford-NV
Copy link
Contributor

Hi stb maintainers!

The goal of this pull request is to modify the fuzzer so that it can catch bugs like #1452. If I've put this together correctly, the CIFuzz automation should fail after finding the PIC crash in #1452 fairly quickly, then succeed once a fix for #1452 has been merged in.

I think there's two reasons why fuzz-testing didn't find issue #1452:

  1. The nullptr dereference within stbi__convert_format only occurs if req_comp is not 0 or 4. Previously, the fuzzing function always set req_comp to 4.
  2. There were no PIC files in the fuzzer seed corpus. The fuzzer may succeed in guessing the first PIC magic number, buy my guess is the second PIC magic number (PICT) 84 bytes later is hard for the fuzzer to guess.

So, this pull request

  1. Uses the last byte of the input (N bytes long with N > 1) to choose the value of req_comp to test with, and passes the initial N-1 bytes to stbi_load_from_memory as the image to read. (Using the last byte of the input as both the last byte of the image and the byte that determines req_comp might be a better idea; I'm not 100% sure about the best approach here.)
  2. Adds a 1x1-pixel PIC file I made to the stb_image tests, and includes it in the fuzzer seed corpus. (This file contains an all-0 comment field, and 1 8bpp uncompressed packet that writes 0s to the RGB fields of the pixel.)

Testing locally, libFuzzer can now find the crash in #1452 in about a second.

Thanks!

…er so that it also searches over the number of requested components
@NBickford-NV
Copy link
Contributor Author

NBickford-NV commented Feb 25, 2023

Looks like this worked — there's the PIC nullptr dereference! https://github.com/nothings/stb/actions/runs/4270488208/jobs/7434315794#step:5:982

================== Job 0 exited with exit code 77 ============
Dictionary: 16 entries
INFO: Running with entropic power schedule (0xFF, 100).
INFO: Seed: 1337
INFO: Loaded 1 modules   (3670 inline 8-bit counters): 3670 [0x663e80, 0x664cd6), 
INFO: Loaded 1 PC tables (3670 PCs): 3670 [0x60d840,0x61bda0), 
INFO:     8606 files found in /github/workspace/cifuzz-corpus/stbi_read_fuzzer
INFO: -max_len is not provided; libFuzzer will not generate inputs larger than 1048576 bytes
INFO: seed corpus: files: 8606 min: 1b max: 1989777b total: 11532072b rss: 41Mb
#4096	pulse  cov: 1104 ft: 2811 corp: 694/12729b exec/s: 151 rss: 232Mb
AddressSanitizer:DEADLYSIGNAL
=================================================================
==39==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x0000005a6206 bp 0x7ffe94771a10 sp 0x7ffe94771[970](https://github.com/nothings/stb/actions/runs/4270488208/jobs/7434315794#step:5:971) T0)
==39==The signal is caused by a READ memory access.
==39==Hint: address points to the zero page.
SCARINESS: 10 (null-deref)
    #0 0x5a6206 in stbi__convert_format(unsigned char*, int, int, unsigned int, unsigned int) /src/stb/tests/../stb_image.h:1785:52
    #1 0x58a9ea in stbi__pic_load(stbi__context*, int*, int*, int*, int, stbi__result_info*) /src/stb/tests/../stb_image.h:6535:11
    #2 0x57e1cd in stbi__load_main(stbi__context*, int*, int*, int*, int, stbi__result_info*, int) /src/stb/tests/../stb_image.h:1159:35
    #3 0x56cbef in stbi__load_and_postprocess_8bit(stbi__context*, int*, int*, int*, int) /src/stb/tests/../stb_image.h:1261:19
    #4 0x57cab7 in stbi_load_from_memory /src/stb/tests/../stb_image.h:1431:11
    #5 0x57cab7 in LLVMFuzzerTestOneInput /src/stb/tests/stbi_read_fuzzer.c:29:26

I was a bit worried about this because I saw a timeout while another run was loading the corpus - looks like there are some JPEG files that specify fewer than 20 million texels (from the limit in stbi_read_fuzzer.c), but take more than 25 seconds to load (likely 60-70 seconds, based on some slow units locally; maybe the 80000000 constant could be reduced to around 10000000?): https://github.com/nothings/stb/actions/runs/4270185040/jobs/7433830850#step:5:148

One interesting thing is that I think it found the nullptr dereference while analyzing the existing ossfuzz corpus. Maybe ossfuzz found a way in the past to synthesize a file with the two PIC magic numbers, and my hypothesis in point 2 about the second magic number being hard for ossfuzz's fuzzers to guess was wrong? (In other words, maybe the only change needed was allowing the requested channels to vary in stbi_read_fuzzer.c?)

@@ -2,21 +2,31 @@
extern "C" {
#endif

#include <stdint.h>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This addition is here to make stbi_read_fuzzer.c compile on Windows: stb_image.h doesn't include <stdint.h> on Windows, but the LLVMFuzzerTestOneInput function signature traditionally uses the uint8_t and size_t types from stdint.h.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant