Potential memory leak in png_handle_pCAL. about libpng HOT 5 CLOSED

Thanks; it looks OK to me. This didn't make it into libpng-1.6.25 which I
released today, so it''ll have to go into the next beta. You can submit a
pull request if you like, but I could just use the referenced patch.

Glenn

On Thu, Sep 1, 2016 at 2:41 PM, dan sinclair [email protected]
wrote:

Given a correctly crafted PNG it is possible that the params passed into
png_set_pCAL fail...

from libpng.

Feel free to grab the patch, otherwise I can put up the pull request on Monday or Tuesday.

from libpng.

It's an API, the caller has to do any required clean-up. Historically png_handle_ has checked the parameters so that png_set_ does not error out, this design is of course totally bogus but it is what it is (I'm working on a new design for 1.7, but it is a major rewrite.)

The rule of thumb is that png_handle_ has to stash all the memory it allocates in png_struct before calling png_set_ and png_handle_pCAL is violating this and allocating memory unnecessarily; it can (and should) stack allocate 'params'.

BTW, even if it wasn't an API a called function should never de-allocate data allocated in the caller, at least not in C.

from libpng.

IRC the same issue was found with sPLT (which is a core chunk, not an extension) and this lead to 'png_chunk_report' which does the right thing. In the past png_set_ was written to call png_warning after the first memory allocation (i.e. inside png_set_) so error handling was somewhat weird. png_chunk_benign_error does not work in png_set_ both because it is not a benign error in write and because, IRC, it doesn't actually build if PNG_NO_READ.

I'm testing a patch that should fix it correctly, without the need to stack-allocate. I've reviewed pngrutil.c; this seems to be the only case left of this bug (though it is very easy to introduce new instances; after all this one was introduced by adding the check on the parameter format in pngset.c without duplicating it in pngrutil.c (all the other checks are repeated in png_handle_pCAL!)

from libpng.

John's patch has been applied to the head of the libpng16 branch of glennrp/libpng; see #135

from libpng.