Revised PNG_CLEANUP to avoid destroying already-destroyed structs #784
Conversation
|
Recent crashes may be due to over-zealous CLEANUP. This patch avoids destroying png structs that have already been destroyed. |
|
Thanks for this change! Is there any chance that this fuzzer file could be included in the upstream libpng repository? |
|
Sure. Would that be considered the master, then, so if we update the file
in libpng repo
it gets updated at oss-fuzz automatically?
…On Mon, Aug 21, 2017 at 7:07 PM, Oliver Chang ***@***.***> wrote:
Thanks for this change!
Is there any chance that this fuzzer file could be included in the
upstream libpng repository?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#784 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/ABe25lvJTnDnz9LmQx3Fc7G3L5Z1KkDnks5sag20gaJpZM4O97Cy>
.
|
|
Yep! We checkout libpng during build, so we can just point our build script to the upstream file instead. Ideally, libpng would also provide build system support (see https://github.com/google/oss-fuzz/blob/master/docs/ideal_integration.md#build-support), but that can come later :) |
|
The licensing comment at the top of libpng_read_fuzzer.cc is a little
ambigous. It talks about a BSD-style
license found in the LICENSE file. But the only LICENSE file I can find is
at the top of oss-fuzz, and that
is Apache-2.0. That's OK with me, but the comments at the top of
libpng_read_fuzzer.cc aren't exactly
what Apache-2.0 requires.
If we move it to the libpng repo I'd rather have it under the libpng
license anyway.
Glenn
…On Mon, Aug 21, 2017 at 7:27 PM, Oliver Chang ***@***.***> wrote:
Yep! We checkout libpng during build, so we can just point our build
script
<https://github.com/google/oss-fuzz/blob/master/projects/libpng/build.sh#L34>
to that file instead.
Ideally, libpng would also provide build system support (see
https://github.com/google/oss-fuzz/blob/master/docs/ideal_
integration.md#build-support), but that can come later :)
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#784 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/ABe25jVtJKx1tvdNf6dscpE-PHiX3MZKks5sahJEgaJpZM4O97Cy>
.
|
|
I've pushed libpng_read_fuzzer.cc to the libpng16 distribution, in
contrib/oss-fuzz
Have a look.
Glenn
|
|
Thanks! I've pushed c1d4fe1 so that we are building the upstream fuzzer, so this PR is no longer needed. |
|
You didn't answer my questions about copyright/license, above. What LICENSE file is the |
|
My bad, missed that. I believe the LICENSE file is referring to the one in chromium, where we copied the file from: https://cs.chromium.org/chromium/src/LICENSE?q=LICENSE&sq=package:chromium&dr I'm afraid I don't know how to answer "If it was talking about the libpng license, that would be great.", but I assume you would need to keep the existing chromium copyright notice as per the LICENSE file |
|
Unfortunately it looks like the new changes have caused a build error: Could you please take a look at this? |
|
Fixed in libpng repo (read_ptr should have been png_handler.png_ptr)
Glenn
…On Mon, Aug 21, 2017 at 8:57 PM, Oliver Chang ***@***.***> wrote:
Unfortunately it looks like the new changes have caused a build error:
Step #3: /src/libpng/contrib/oss-fuzz/libpng_read_fuzzer.cc:94:18: error: use of undeclared identifier 'read_ptr'
Could you please take a look at this?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#784 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/ABe25hviTJEnyZ_9pmu0Xcryab5rH6X3ks5said_gaJpZM4O97Cy>
.
|
|
Updated the README.txt and libpng_read_fuzzer.cc in contrib/oss-fuzz to
point to the Chromium LICENSE.
On Mon, Aug 21, 2017 at 9:21 PM, Glenn Randers-Pehrson <[email protected]>
wrote:
… Fixed in libpng repo (read_ptr should have been png_handler.png_ptr)
Glenn
On Mon, Aug 21, 2017 at 8:57 PM, Oliver Chang ***@***.***>
wrote:
> Unfortunately it looks like the new changes have caused a build error:
>
> Step #3: /src/libpng/contrib/oss-fuzz/libpng_read_fuzzer.cc:94:18: error: use of undeclared identifier 'read_ptr'
>
> Could you please take a look at this?
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <#784 (comment)>, or mute
> the thread
> <https://github.com/notifications/unsubscribe-auth/ABe25hviTJEnyZ_9pmu0Xcryab5rH6X3ks5said_gaJpZM4O97Cy>
> .
>
|
|
Thanks! Confirmed that the build is working again. |
No description provided.