Allow a zero sized iCCP chunk.

[bobsayshilol]

This fixes two issues one issue:

1. malloc(0) is allowed to return NULL as a value, so we shouldn't always be counting a NULL return value from malloc() as an allocation error.
   2) A memcpy() with a nullptr as the source or destination is undefined behaviour, even if the length to copy is zero.

I don't see anything specifically disallowing a zero sized iCCP chunk in the PNG spec, however the behaviour of LodePNG would have been inconsistent depending on whether or not the allocator returned NULL for a zero sized allocation or not.

[bobsayshilol]

Rebased and simplified the changes after the introduction of lodepng_memcpy().

[lvandeve]

The relevant spec is here:
http://www.libpng.org/pub/png/spec/1.2/PNG-Chunks.html#C.iCCP

When taking the spec really really literally, it does say:
"The color space of the ICC profile must be an RGB color space for color images (PNG color types 2, 3, and 6), or a monochrome grayscale color space for grayscale images (PNG color types 0 and 4). "
A zero-length ICC profile is neither of those so that would be invalid :)

I agree that the current code is not great due to not handling this issue separately (but returning an error due to an allocation check instead). It could return it as a separate error (maybe with error code 100, or with a new code), or allow the 0-length ICC profiles.

Whether 0-length ICC chunk should actually be allowed is a different issue though. Do you have a use case or know existing cases where such chunks occur?

Thank you :)

P.S. since the code to read the iCCp chunk was based on the code to read zTXt chunks, zTXt chunks are likely giving the same error, and so are iTXt and tEXt chunks, I didn't test it yet but probably they wouldn't support it, and for text chunks the spec does say explicitly that 0 lengths should be allowed. So those must be fixed for sure (if they're indeed broken), thanks for finding this!

[lvandeve]

The spec says the profile conforms to ICC (without "must"), but due to its usage of the words "must" after that for RGB or GREY profile, I'd say, best to handle it as an error, unless you know there are use cases that need it. Would you like to change the merge request to make this case return e.g. error 100?

[bobsayshilol]

Ah I misread the API to be a key-value pair style thing with names as the keys and profile data as the values, so I assumed it was extensible and a client could add new names as they wanted which may not necessarily have had a sized profile. Your explanation clears things up, thanks!

I don't have any examples as I only spotted this since it was right next to the nullptr memcpy() issue I caught from fuzzing, so it would have been bogus data anyway. I'll add those error codes now.

Also looking at the uses of lodepng_malloc() the other chunks seem OK, however I did notice the allocation in addChunk_tIME() which looks unnecessary to me since an array of 7 chars would fit nicely on the stack?

[lvandeve]

Thanks!

You're right about addChunk_tIME, I'll reduce some allocations soon