Re: master 8ff7338fdd0 1/4: When debugging image.c, abort if silent truncation

[Stefan Kangas]

Paul Eggert <eggert@cs.ucla.edu> writes:

> branch: master
> commit 8ff7338fdd05fe6d21765711327a99c87cfd7613
> Author: Paul Eggert <eggert@cs.ucla.edu>
> Commit: Paul Eggert <eggert@cs.ucla.edu>
>
>     When debugging image.c, abort if silent truncation
>
>     * src/image.c (image_build_heuristic_mask, png_load_body):
>     Abort if snprintf truncated.  (If truncation is not possible
>     here we should use sprintf instead, as that simplifies
>     automatic runtime checking.)

Thanks for adding the eassert here.

I don't think I understand the above comment, however: I understand that
some tools might detect an out-of-bounds write with sprintf but not with
snprintf.  But the eassert you added should do that job too, so what
would be the reasons for still preferring sprintf?

I agree that an out-of-bounds write seems quite unlikely with the code
we have now: I can't see how it could happen.  Despite that, I
personally feel more comfortable with always doing proper bounds
checking in image processing code above only doing it in special cases
and risking a mistake.  This code runs on arbitrary data found online,
or even sent in emails and automatically displayed due to (wrong and
careless) user settings, and so on.