bmp: left shift by 24 bits does not always fit in an "int" (clrUsed)
@tobiasmue
Submitted by Tobias Mueller Link to original bug (#776695)
Description
io-bmp.c:335:28: runtime error: left shift of 222 by 24 places cannot be represented in type 'int'
io-bmp.c:337:26: runtime error: shift exponent 32 is too large for 32-bit type 'int'
(process:20881): GLib-GObject-WARNING **: value "-2147352580" of type 'gint' is invalid or out of range for property 'rowstride' of type 'gint'
fish: “./tests/pixbuf-read /tmp/comput…” terminated by signal SIGTRAP (Trace or breakpoint trap)
We can see the offending shift here:
if (State->Header.size == 12)
clrUsed = 1 <`< State->`Header.depth;
else
clrUsed = (BIH[35] << 24) + (BIH[34] << 16) + (BIH[33] << 8) + (BIH[32]);
if (clrUsed > (1U <`< State->`Header.depth))
{
g_set_error_literal (error
The values get promoted to (signed) ints. If you shift by 24 digits and the high bit is set, then you overflow the range of the signed int. So the BIH[35], which gets promoted to an int, should be marked as unsigned before shifting. But also, n_colors is assigned the value of clrUsed later and n_colors seems to be an unsigned type:
struct headerpair {
guint32 size;
gint32 width;
gint32 height;
guint depth;
guint Negative; /* Negative = 1 -> top down BMP,
Negative = 0 -> bottom up BMP */
guint n_colors;
};
So I guess that clrUsed should also be unsigned. That would allow a shift of 1 by up to 31 digits.
We cannot shift a 1 by 32 places in a regular 32bit int, being it unsigned or not. So I think the pragmatic approach is to check for the depth being strictly less than 31. I don't know whether this breaks some images. I guess it doesn't, though.
This seems to also be CID 1388521.
Version: git master