pnggroup / libpng

LIBPNG: Portable Network Graphics support, official libpng repository
http://libpng.sf.net
Other
1.25k stars 611 forks source link

Possible miscalculation of buffer length in `png_icc_profile_error` #553

Open johnstiles-google opened 4 months ago

johnstiles-google commented 4 months ago

Noticed a strange/misleading comment in png.c.

pos = png_safecat(message, (sizeof message), pos, "h: "); /* +2 = 116 */

The string being appended, h:, is three characters long—not two. In practice this may be a non-issue, because the next line allows an arbitrary +79 slop factor which doesn't seem to correspond to any real limit.

/* The 'reason' is an arbitrary message, allow +79 maximum 195 */

jbowler commented 4 months ago

That's why I wrote safecat; people do this all the time. I.e. update a message and don't increase the length of the corresponding buffer or do the addition wrong. It's safe because this is precisely what safecat checks for (it will never overwrite the buffer).

In this case the comment error was in the original code (871b1d). The comment should be fixed some time; remarkable that so much copy editing was done including changes to the comment but one one until now noticed the words in the comment were wrong!

johnstiles-google commented 4 months ago

Yes, since this is safecat, the only real danger is truncation; there shouldn't be any security/UB risk.