search

pnggroup / libpng

LIBPNG: Portable Network Graphics support, official libpng repository
http://libpng.sf.net
Other
1.29k stars 626 forks source link

improve libpng coverage on OSS-Fuzz #274

Open kcc opened 5 years ago

kcc commented 5 years ago

requesting help with gaining more coverage on OSS-Fuzz, details here: https://github.com/google/oss-fuzz/issues/2111

kcc commented 5 years ago

My first attempt to improve the coverage: https://github.com/google/oss-fuzz/pull/2114 Please give feedback, and consider taking over this fuzz target. It very quickly finds a memory leak:

==1==ERROR: LeakSanitizer: detected memory leaks
--
  | Direct leak of 256 byte(s) in 1 object(s) allocated from:
  | #0 0x4efdcf in malloc _asan_rtl_
  | #1 0x55c0cd in OSS_FUZZ_png_malloc libpng/pngmem.c:179:10
  | #2 0x599de2 in OSS_FUZZ_png_set_tRNS libpng/pngset.c:1019:35
  | #3 0x58a79c in OSS_FUZZ_png_handle_tRNS libpng/pngrutil.c:1914:4
  | #4 0x55ceac in OSS_FUZZ_png_read_info libpng/pngread.c:245:10
  | #5 0x55fae6 in OSS_FUZZ_png_read_png libpng/pngread.c:1052:4
  | #6 0x538b83 in LLVMFuzzerTestOneInput /src/libpng_transforms_fuzzer.cc:100:5
kcc commented 5 years ago

The leak above is fixed. Now the blocker for further fuzzing is #275

kcc commented 5 years ago

@sboukortt thanks for the leak fix. Maybe you can suggest improvements to my experimental PNG fuzz target? https://github.com/google/oss-fuzz/blob/master/projects/libpng-proto/libpng_transforms_fuzzer.cc

The goal is to be able to cover more parts of the libpng code. The one we currently have in libpng trunk (https://github.com/glennrp/libpng/tree/libpng16/contrib/oss-fuzz) is significantly weaker than the new one, but there is still lots to improve.

LocutusOfBorg commented 5 years ago

can you please try again with the patches I mentioned here? https://github.com/glennrp/libpng/issues/275#issuecomment-481162675

kcc commented 5 years ago

Err. What do you mean by "try again"? The fuzzing runs automatically and picks up the fresh master every day. (I've just realized that asan has been disabled on oss-fuzz due to #275, I've enabled it back.

LocutusOfBorg commented 5 years ago

ok, nice! I was not aware of the continuous integration against master... :)

kcc commented 5 years ago

FTR, here is the coverage report for the libpng fuzzers that we run 24/7

Vanilla (using the upstream fuzz target): https://oss-fuzz.com/coverage-report/job/libfuzzer_asan_libpng/latest Experimental: https://oss-fuzz.com/coverage-report/job/libfuzzer_asan_libpng-proto/latest

In both cases large chunks of code (e.g. in pngtran.c) remain uncovered.

ctruta commented 5 years ago

In both cases large chunks of code (e.g. in pngtran.c) remain uncovered.

Thank you. Most of those transforms have existed ever since libpng-1.0, when the libpng folks thought it was something good to have; but then, I don't know how many users actually needed it, and how many programs actually tested it.

I still have to focus on existing bugs for the next version, but, eventually (and slowly, considering my very limited spare time), I will write new tests. Either that, or altogether remove the code that is untested and was broken since forever-ago.

kcc commented 5 years ago

Thank you! When writing tests, pleeeease also consider adding functionality to https://github.com/glennrp/libpng/blob/libpng16/contrib/oss-fuzz/libpng_read_fuzzer.cc Removing unused/untested code is obviously even better.

Are you aware of https://www.google.com/about/appsecurity/patch-rewards/ ? It explicitly mentions libpng: 

Core infrastructure data parsers: libjpeg, libjpeg-turbo, libpng, giflib, zlib, libxml2
...
Rewards for qualifying submissions range from $500 to $20,000.
tangyaofang commented 5 years ago

I want to know how to reproduce this problem , how libpng16/contrib/oss-fuzz should execute the test.

thealberto commented 2 years ago

Hello everybody, I would like to try to participate helping to improve the coverage. Initailly I would like to start just adding new png files to the corpora. unfortunately ATM i'm not allowed to get access to the corpora since I'm not listed inside the project.yaml file. Is there any possiblity that someone could make the files available to me?

Thanks

@kcc

kcc commented 2 years ago

Please send a pull request adding yourself to https://github.com/google/oss-fuzz/blob/master/projects/libpng/project.yaml

thealberto commented 2 years ago

Hi, under the auto_ccs? Can you confirm that there is not difference with the images stored here.

Thanks

kcc commented 2 years ago

yes, auto_ccs.

Can you confirm that there is not difference with the images stored here

Not sure I understand

thealberto commented 2 years ago

yes, auto_ccs.

Can you confirm that there is not difference with the images stored here

Not sure I understand

The link points to to a directory where there are png files which give a similar coverage to what currently is possible via oss-fuzz. I was wondering if the corpora is the same.

PR submitted 7938

thealberto commented 2 years ago

Hello, I need a confirmaion from the maintainer. On the mentioned above PR. Anyone can help?

@ctruta

thealberto commented 2 years ago

Hi @kcc , I have worked a little bit on it and improve the coverage of the fuzzer within pngtrans.c. It isn't perfect but better:

immagine

A quick example with the new fuzzer lead to the following heap-overflow:

==191008==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x61b000000680 at pc 0x0000004d715a bp 0x7ffc80d5d8b0 sp 0x7ffc80d5d078
WRITE of size 2048 at 0x61b000000680 thread T0
    #0 0x4d7159 in __asan_memcpy (/.../libpng-1.6.37-fsanitize_fuzzer+0x4d7159)
    #1 0x570075 in png_combine_row /.../oss/BUILD/pngrutil.c:3678:4

It might be a duplicate but so far I haven't done any additional investigation.

I'm happy to continue the research and improve the coverage but I need some confirmation that I'm going in the right direct and that my PR can be accepted.

Hopefully someone will help us soon

kcc commented 2 years ago

@kcc could you help us to understand how to achieve the same level of coverage?

(in https://github.com/glennrp/libpng/issues/419)

not sure what the question is

thealberto commented 2 years ago

@kcc could you help us to understand how to achieve the same level of coverage?

(in #419)

not sure what the question is

Hi @kcc , Currently the coverage of the fuzzer I ran locally is much worse than the coverage achieved by oss-fuzz. Could you help me to understand why? Do you think it is only related to the corpus? I should be able to access to it now anyway .. my PR on the oss-fuzz has been merged.

Thanks

kcc commented 2 years ago

yes, you will now have access to the corpus (maybe, give it a few hours?). With the corpus, you should see the same coverage. But is your question about how to get to that coverage from a small seed corpus?

thealberto commented 2 years ago

yes, you will now have access to the corpus (maybe, give it a few hours?). With the corpus, you should see the same coverage.

Sure, I'll try tomorrow.

But is your question about how to get to that coverage from a small seed corpus?

I just want to achieve the same level of coverage but not from a small corpus. Again, I think the official corpus will help me.

Thanks

ctruta commented 2 years ago

FYI, I integrated into the master branch the fix submitted in https://github.com/glennrp/libpng/pull/279

thealberto commented 1 year ago

Hi @ctruta , since you are back now I would like to talk about this issue. Ideally I would like to greatly increase the coverage of the fuzzer so hopefully more bugs could be found.

Is there any specific area that you would like to cover more specifically? Do you have in mind a specific structure for it?

Thanks

ctruta commented 10 months ago

I was "back", and then "not back", and then "back" again, and then "not back" again...... Happy New Year -- please ping me if you're still interested.

thealberto commented 10 months ago

Hi @ctruta , Happy to see that you are back and that the project is alive.

I'm more than happy to work on it. Please have a look at https://github.com/glennrp/libpng/issues/450 as well.

How would you like to proceed? Anything specific that you want to do first?

Thanks

ctruta commented 10 months ago

@thealberto well, I have a lot of things to catch up with, all over the place. However, if you send me a new PR, (or if you point to me an old one that I might have missed), preferably (but not necessarily) at https://github.com/pnggroup/libpng/ then I'll be happy to review and integrate.

thealberto commented 10 months ago

@ctruta Yes sure no problem. I didn't realise that the repo has been migrated.

Thanks