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

integrate with OSS-Fuzz (continuous automated fuzzing), and fix a recent regression (leak) #170

Closed
kcc opened this issue Aug 1, 2017 · 18 comments

Comments

@kcc
Copy link
Contributor

kcc commented Aug 1, 2017

Hello,

OSS-Fuzz is a continuous fuzzing service provided to important OSS projects for free:
https://github.com/google/oss-fuzz
https://opensource.googleblog.com/2017/05/oss-fuzz-five-months-later-and.html

Normally, an open-source project would do the integration with OSS-Fuzz themselves, but since libpng is so widely used we've made an initial integration for libpng ourselves a while ago:
https://github.com/google/oss-fuzz/tree/master/projects/libpng

The fuzzer has been running since late 2016 w/o any bug reports, but now it has reported a first bug (memory leak), which seems to be a recent regression, introduced between 4ab78af and 68a80b1

ERROR: LeakSanitizer: detected memory leaks
Direct leak of 538976288 byte(s) in 1 object(s) allocated from:
    #0 0x4de2c8 in malloc /src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:67
    #1 0x5a4406 in png_malloc_warn /src/libpng/pngmem.c:217:23
    #2 0x5d2689 in png_handle_eXIf /src/libpng/pngrutil.c:2031:15
    #3 0x5a4d34 in png_read_info /src/libpng/pngread.c:180:10
    #4 0x5190f0 in LLVMFuzzerTestOneInput /src/libpng_read_fuzzer.cc:88:3

So, at this moment we would ask for your help to better integrate libpng with OSS-Fuzz:

@glennrp
Copy link
Contributor

glennrp commented Aug 1, 2017 via email

@kcc
Copy link
Contributor Author

kcc commented Aug 1, 2017

I think I generated the appropriate pull request

Nope, I don't see it. :(

@kcc
Copy link
Contributor Author

kcc commented Aug 1, 2017

Without seeing the test case I attempted a fix which I've pushed to libpng16.

Didn't seem to help.
I can attach the repro here (should I?), but it only triggers in read_fuzzer.cc (which does png_set_crc_action(png_handler.png_ptr, PNG_CRC_QUIET_USE, PNG_CRC_QUIET_USE);)

@glennrp
Copy link
Contributor

glennrp commented Aug 1, 2017

You can attach the reproducer here or mail it to [email protected]

@kcc
Copy link
Contributor Author

kcc commented Aug 1, 2017

Attached reproducer (gzip-ed, to please GitHub).
It causes the leak when fed to read_fuzzer.cc

clusterfuzz-testcase-minimized-6064109344784384.gz

@glennrp
Copy link
Contributor

glennrp commented Aug 2, 2017 via email

@kcc
Copy link
Contributor Author

kcc commented Aug 2, 2017

I can confirm that the leak report is gone, thanks!
Do you mind if I add your e-mail to https://github.com/google/oss-fuzz/blob/master/projects/libpng/project.yaml ?

This way you will receive e-mail if the bot finds any more bugs (and you'll get access to the details)

@glennrp
Copy link
Contributor

glennrp commented Aug 2, 2017

Yes, please add my e-mail to the distribution list.

FYI I tested with
valgrind pngtest --relaxed file.png
The "--relaxed" option turns off CRC checking.

kcc added a commit to google/oss-fuzz that referenced this issue Aug 2, 2017
@kcc
Copy link
Contributor Author

kcc commented Aug 2, 2017

Done: https://github.com/google/oss-fuzz/blob/master/projects/libpng/project.yaml

When you have a chance, please take a look at https://github.com/google/oss-fuzz/blob/master/projects/libpng/libpng_read_fuzzer.cc (is there anything to improve there? can you add something similar to the main libpng tree?).

Note that Google has a monetary reward program for participating in OSS-Fuzz:
https://opensource.googleblog.com/2017/05/oss-fuzz-five-months-later-and.html

The "--relaxed" option turns off CRC checking.

Good to know, thanks!

@glennrp
Copy link
Contributor

glennrp commented Aug 3, 2017

I gave a whack at updating the project, by adding the eXIf chunk to the dictionary.
Where is the seed corpus actually located? Is it the PNG files contained in the libpng distribution?

@Dor1s
Copy link

Dor1s commented Aug 3, 2017

Yes, @glennrp, current seed corpus is being collected by the following command:

find $SRC/libpng -name "*.png" | xargs zip $OUT/libpng_read_fuzzer_seed_corpus.zip

in libpng checkout.

@Dor1s
Copy link

Dor1s commented Aug 3, 2017

There also is another set of corpus files generated by the fuzzer and improved over time. Those files in the cloud are not "seed" corpus, it's a working corpus, I would say. It is synchronized across all VMs running libpng fuzzer, and it grows automatically over time.

You can download a minimized version of that corpus using "Corpus backup" link on the fuzzer stats page: https://oss-fuzz.com/v2/fuzzer-stats/by-day/2017-07-27/2017-08-02/fuzzer/libFuzzer_libpng_read_fuzzer/job/libfuzzer_asan_libpng

@kcc
Copy link
Contributor Author

kcc commented Aug 3, 2017

If you know some other good public source of diverse png files, feel free to extend the command line that creates libpng_read_fuzzer_seed_corpus.zip in https://github.com/google/oss-fuzz/blob/master/projects/libpng/build.sh

@txtsd
Copy link

txtsd commented Sep 1, 2017

@kcc Could you clarify what you mean by diverse? I can try to find a source.

@glennrp
Copy link
Contributor

glennrp commented Sep 1, 2017

I assumed @kcc meant a large variety of PNG color_types, sample_depths, etc. The libpng directory "contrib/testpngs" which is currently included in the corpus covers most of them.

@kcc
Copy link
Contributor Author

kcc commented Sep 1, 2017

Yes. By diverse I mean different features of the data format, or, more precisely, inputs that cover different parts of code.
I encourage you to look at the coverage achieved on the oss-fuzz's corpus to see what code remains uncovered.

@glennrp did you change something in the build system recently?
Our dashboard (oss-fuzz.com, you can see it too) shows that the last two days the total number of instrumented basic blocks dropped from 6634 to 1166

@glennrp
Copy link
Contributor

glennrp commented Sep 1, 2017

I did update libpng_read_fuzzer.cc but the "coverage" report is showing me some other version.

@kcc
Copy link
Contributor Author

kcc commented Sep 1, 2017

Moving the discussion to google/oss-fuzz#809, if you don't mind

@glennrp glennrp closed this as completed Sep 22, 2017
tmatth pushed a commit to tmatth/oss-fuzz that referenced this issue Oct 22, 2018
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

No branches or pull requests

4 participants