Crashing in read_png() with RGB images on X11

[laddershoe]

Hey, I ran into an issue (on Ubuntu) that seems related to some stuff in #31.

When I copy an RGB image (I've been testing with Lena), Firefox will copy it as RGBA, and clip handles it fine. Chrome will copy it as RGB, and clip crashes somewhere inside png_destroy_read_struct(). I can reliably repro this inside the show_image.cpp example if I put the entire program in a loop so we're calling clip::get_image() more than one time (sometimes it happens the first time you call get_image, but twice seems to reliably crash).

I think memory is getting clobbered in read_png() in clip_x11_png.h. In the image::image(const image_spec& spec) constructor, it allocates spec.bytes_per_row*spec.height bytes, which for an RGB image is 3 bytes/pixel. But in read_png(), img.data() is getting cast to a uint32_t* and the writing is done by advancing the pointer, so the function is writing 4 bytes/pixel and thus writing off the end of the allocation. Memory gets clobbered and the result is a spurious crash in libpng.

For my use case, I can fix the crash in the image::image(const image_spec& spec) constructor
by changing m_data(new char[spec.bytes_per_row*spec.height]), to
m_data(new char[spec.width*spec.height*4]) so it's always allocating 4 bytes/pixel for the dest image regardless of what's in the source image.

No idea if this is the right fix though. Can you advise? I'm happy to leave this in your hands, or I can take it further if you like. Just let me know what would be best for you.

Thanks for this great library :)

[dacap]

Hi @laddershoe, thanks for the bug report (and I'm glad the library is useful for you ❤️)

I'll take a look to this issue right now, I was able to reproduce it with the memory sanitizer on (-fsanitizer=address).

[dacap]

This issue should be fixed now in the main branch 👍

[laddershoe]

Just realized (to my embarrassment) that I never closed the loop on this... this is confirmed fixed for me. Thanks so much, and if you have a link where I can throw you a couple of bucks, I'd be happy to do so :)

[dacap]

Thank you for reporting this bug, and don't worry, I don't have a link for donations but if someday you want to collaborate with us we have a program using this library: https://www.aseprite.org/

Probably in the future if GitHub add sponsorship support for Argentina, I'll be able to enable it.