Skip to content
This repository has been archived by the owner on Nov 28, 2019. It is now read-only.

Heap Buffer Overflow Vulnerability in Pngdefry #1

Closed
RajUllas opened this issue Mar 22, 2017 · 5 comments
Closed

Heap Buffer Overflow Vulnerability in Pngdefry #1

RajUllas opened this issue Mar 22, 2017 · 5 comments

Comments

@RajUllas
Copy link

This is to report a heap overflow vulnerability in Pngdefry. This issue affects the 'process()' function of the 'pngdefry.c' source file.

Valgrind reports Invalid write of size 1 and Invalid read of size 1.

To reproduce this issue open the 'png-file' with Pngdefry application.

POC files attached below:

Addresssanitizer.txt
GDB.txt
png_file.txt
Valgrind.txt

@Tatsh
Copy link
Owner

Tatsh commented Mar 23, 2017

Thanks for this. I am maintaining this fork but I think you should definitely report this to the original author too (email is in the README).

@RajUllas
Copy link
Author

Thanks for the response.
Yes, I have reported it to author also.

Tatsh added a commit that referenced this issue Mar 23, 2017
See #1
This does not fix for the vulnerability when the checksums are
valid.

`-C` switch allows for old behaviour.
@Tatsh
Copy link
Owner

Tatsh commented Mar 23, 2017

Quick workaround for now is to not ignore when the CRC32 check fails on the IHDR as it does with your example file (switch -C will force ignoring it). You would only have to correct your CRC32 in the header to trigger the vulnerability. Still, I think ignoring a bad CRC32 is a bad idea and the default behaviour ought to be to not process such a file.

Tatsh added a commit that referenced this issue Mar 23, 2017
See #1
This does not fix for the vulnerability when the checksums are
valid.

`-C` switch allows for old behaviour.
@Tatsh
Copy link
Owner

Tatsh commented Mar 23, 2017

Closing this. Invalid CRC32 no longer ignored by default. This is as per recommendation:

It is strongly recommended that decoders should verify the CRC on each chunk.

https://www.w3.org/TR/PNG-Decoders.html#D.Error-checking

@Tatsh Tatsh closed this as completed Mar 23, 2017
@Jongware
Copy link

Jongware commented Apr 2, 2017

This issue has been addressed per your recommendation (not auto-repair bad CRC's, adding a flag to explicitly allow this), as well as the underlying cause (documented in my web page). A new version 1.2 has been put on my site.

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

No branches or pull requests

3 participants