old
/
legacy-imlib2
3
0
Fork 4
Code Issues 3 Pull Requests Projects Releases Wiki Activity

bmp fixes 

SVN revision: 21477
This commit is contained in:
Carsten Haitzler 2006-03-22 02:08:39 +00:00
parent 8e83a0d122
commit 100ee8171d
1 changed files with 195 additions and 150 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

345
src/modules/loaders/loader_bmp.c
View File

@ -3,6 +3,12 @@
* imlib's old BMP loader * imlib's old BMP loader
*/ */
/*
* 21.3.2006 - Changes made by Petr Kobalicek
* - Simplify and make secure RLE encoding
* - Fix 16 and 32 bit depth (old code was incorrect and it's commented)
*/
#ifdef HAVE_CONFIG_H #ifdef HAVE_CONFIG_H
# include <config.h> # include <config.h>
#endif #endif
@ -31,6 +37,13 @@ typedef struct tagRGBQUAD {
#define BI_RLE4 2 #define BI_RLE4 2
#define BI_BITFIELDS 3 #define BI_BITFIELDS 3
/* 21.3.3006 - Use enumeration for RLE encoding. This makes it more readable */
enum {
RLE_NEXT = 0, /* Next line */
RLE_END = 1, /* End of RLE encoding */
RLE_MOVE = 2 /* Move by X and Y (Offset is stored in two next bytes) */
};
static int static int
ReadleShort(FILE * file, unsigned short *ret) ReadleShort(FILE * file, unsigned short *ret)
{ {
@ -122,6 +135,12 @@ load(ImlibImage * im, ImlibProgressFunction progress,
unsigned long rmask = 0xff, gmask = 0xff, bmask = 0xff; unsigned long rmask = 0xff, gmask = 0xff, bmask = 0xff;
unsigned long rshift = 0, gshift = 0, bshift = 0; unsigned long rshift = 0, gshift = 0, bshift = 0;
/*
* 21.3.2006:
* Added these two variables for RLE.
*/
unsigned char byte1, byte2;
if (im->data) if (im->data)
return 0; return 0;
f = fopen(im->real_file, "rb"); f = fopen(im->real_file, "rb");
@ -186,12 +205,12 @@ load(ImlibImage * im, ImlibProgressFunction progress,
fclose(f); fclose(f);
return 0; return 0;
} }
if ((w < 1) || (h < 1) || (w > 8192) || (h > 8192)) if ((w < 1) || (h < 1) || (w > 8192) || (h > 8192))
{ {
fclose(f); fclose(f);
return 0; return 0;
} }
if (bitcount < 16) if (bitcount < 16)
{ {
@ -221,12 +240,9 @@ load(ImlibImage * im, ImlibProgressFunction progress,
ReadleLong(f, &rmask); ReadleLong(f, &rmask);
for (bit = bitcount - 1; bit >= 0; bit--) for (bit = bitcount - 1; bit >= 0; bit--)
{ {
if (bmask & (1 << bit)) if (bmask & (1 << bit)) bshift = bit;
bshift = bit; if (gmask & (1 << bit)) gshift = bit;
if (gmask & (1 << bit)) if (rmask & (1 << bit)) rshift = bit;
gshift = bit;
if (rmask & (1 << bit))
rshift = bit;
} }
} }
else if (bitcount == 16) else if (bitcount == 16)
@ -326,102 +342,116 @@ load(ImlibImage * im, ImlibProgressFunction progress,
} }
} }
} }
/*
* 21.3.2006
* Bug fixes and optimization:
*
* RLE encoding is dangerous and can be used by attackers by creating special files.
* We has 'buffer_ptr' and 'buffer_end' variables and buffer_end points to first
* unaccessible byte in buffer.
* - If we use 'byte = *(buffer_ptr++) in main loop we must check if
* 'buffer_ptr != buffer_end', because special or incomplete bmp file can generate
* segfault (I was writing it, because in RLE we need to read depending count of
* bytes that depends on requester operation).
* SOLUTION: Don't read one byte, read two bytes and check.
* - If RLE teels us than single color length will be larger than allowed, we can
* stop, because bitmap is corrupted or crawled.
* SOLUTION: Check for length ('l' variable in RLE) and break loop if it's invalid
* IMPROVEMENTS: We can stop checking if 'x' is out of rangle, because it never be.
* - In RLE4 use one bigger loop that fills two pixels. This is faster and cleaner.
* If one pixel remains (the tail), do it on end of the loop.
* - If we will check x and y (new line and skipping), we can't go outsize imlib
* image buffer.
*/
if (bitcount == 4) if (bitcount == 4)
{ {
if (comp == BI_RLE4) if (comp == BI_RLE4)
{ {
/*
* 21.3.2006: This is better than using 'if buffer_ptr + 1 < buffer_end'
*/
unsigned char *buffer_end_minus_1 = buffer_end - 1;
x = 0; x = 0;
y = 0; y = 0;
for (i = 0, g = 1; for (i = 0; i < imgsize && buffer_ptr < buffer_end_minus_1; i++)
i < imgsize && g && buffer_ptr < buffer_end; i++)
{ {
byte = *(buffer_ptr++); byte1 = buffer_ptr[0];
if (byte) byte2 = buffer_ptr[1];
buffer_ptr += 2;
if (byte1)
{ {
unsigned char t1, t2; DATA32 t1, t2;
l = byte; l = byte1;
byte = *(buffer_ptr++); /* Check for invalid length */
t1 = byte & 0xF; if (l + x > w) goto _bail;
t2 = (byte >> 4) & 0xF;
for (j = 0; j < l; j++)
{
k = (j & 1) ? t1 : t2;
if (x >= w) t1 = 0xff000000 | (rgbQuads[byte2 >> 4].rgbRed << 16) |
break; (rgbQuads[byte2 >> 4].rgbGreen << 8) |
(rgbQuads[byte2 >> 4].rgbBlue ) ;
*ptr++ = 0xff000000 | t2 = 0xff000000 | (rgbQuads[byte2 & 0xF].rgbRed << 16) |
(rgbQuads[k].rgbRed << 16) | (rgbQuads[byte2 & 0xF].rgbGreen << 8) |
(rgbQuads[k].rgbGreen << 8) | (rgbQuads[byte2 & 0xF].rgbBlue ) ;
rgbQuads[k].rgbBlue; for (j = l/2; j; j--) {
x++; ptr[0] = t1;
if (ptr > data_end) ptr[1] = t2;
ptr = data_end; ptr += 2;
} }
/* tail */
if (l & 1) *ptr++ = t1;
x += l;
} }
else else
{ {
byte = *(buffer_ptr++); switch (byte2)
switch (byte)
{ {
case 0: case RLE_NEXT:
x = 0; x = 0;
y++; if (++y >= h) goto _bail;
ptr = im->data + ((h - y - 1) ptr = im->data + (h - y - 1) * w;
* w * sizeof(DATA32));
if (ptr > data_end)
ptr = data_end;
break; break;
case 1: case RLE_END:
g = 0; goto _bail;
break; case RLE_MOVE:
case 2: /* Need to read two bytes */
x += *(buffer_ptr++); if (buffer_ptr >= buffer_end_minus_1) goto _bail;
y += *(buffer_ptr++); x += buffer_ptr[0];
ptr = im->data + ((h - y - 1) * w * y += buffer_ptr[1];
sizeof(DATA32)) + x; buffer_ptr += 2;
if (ptr > data_end) /* Check for correct coordinates */
ptr = data_end; if (x >= w) goto _bail;
if (y >= h) goto _bail;
ptr = im->data + (h - y - 1) * w + x;
break; break;
default: default:
l = byte; l = byte2;
for (j = 0; j < l; j++) /* Check for invalid length and valid buffer size */
{ if (l + x > w) goto _bail;
char t1 = '\0', t2 = if (buffer_ptr + (l >> 1) + (l & 1) > buffer_end) goto _bail;
'\0';
if ((j & 1) == 0) for (j = l/2; j; j--) {
{ byte = *buffer_ptr++;
byte = *(buffer_ptr++); ptr[0] = 0xff000000 | (rgbQuads[byte >> 4].rgbRed << 16) |
t1 = byte & 0xF; (rgbQuads[byte >> 4].rgbGreen << 8) |
t2 = (byte >> 4) & 0xF; (rgbQuads[byte >> 4].rgbBlue ) ;
} ptr[1] = 0xff000000 | (rgbQuads[byte & 0xF].rgbRed << 16) |
k = (j & 1) ? t1 : t2; (rgbQuads[byte & 0xF].rgbGreen << 8) |
(rgbQuads[byte & 0xF].rgbBlue ) ;
if (x >= w) ptr += 2;
{ }
buffer_ptr += (l - j) / 2; if (l & 1) {
break; byte = *buffer_ptr++;
} *ptr++ = 0xff000000 | (rgbQuads[byte >> 4].rgbRed << 16) |
(rgbQuads[byte >> 4].rgbGreen << 8) |
*ptr++ = 0xff000000 | (rgbQuads[byte >> 4].rgbBlue ) ;
(rgbQuads[k].rgbRed << 16) | }
(rgbQuads[k].rgbGreen << 8) | x += l;
rgbQuads[k].rgbBlue;
x++;
if (ptr > data_end)
ptr = data_end;
}
if ((l & 3) == 1) if ((l & 3) == 1)
{ buffer_ptr += 2;
tempchar = *(buffer_ptr++);
tempchar = *(buffer_ptr++);
}
else if ((l & 3) == 2) else if ((l & 3) == 2)
buffer_ptr++; buffer_ptr++;
break; break;
@ -497,78 +527,63 @@ load(ImlibImage * im, ImlibProgressFunction progress,
{ {
if (comp == BI_RLE8) if (comp == BI_RLE8)
{ {
/*
* 21.3.2006: This is better than using 'if buffer_ptr + 1 < buffer_end'
*/
unsigned char *buffer_end_minus_1 = buffer_end - 1;
x = 0; x = 0;
y = 0; y = 0;
for (i = 0, g = 1; for (i = 0; i < imgsize && buffer_ptr < buffer_end_minus_1 && g; i++)
i < imgsize && buffer_ptr < buffer_end && g; i++)
{ {
byte = *(buffer_ptr++); byte1 = buffer_ptr[0];
if (byte) byte2 = buffer_ptr[1];
buffer_ptr += 2;
if (byte1)
{ {
l = byte; DATA32 pix = 0xff000000 | (rgbQuads[byte2].rgbRed << 16) |
byte = *(buffer_ptr++); (rgbQuads[byte2].rgbGreen << 8) |
for (j = 0; j < l; j++) (rgbQuads[byte2].rgbBlue ) ;
{ l = byte1;
if (x >= w) if (x + l > w) goto _bail;
break; for (j = l; j; j--) *ptr++ = pix;
x += l;
*ptr++ = 0xff000000 |
(rgbQuads[byte].rgbRed << 16) |
(rgbQuads[byte].rgbGreen << 8) |
rgbQuads[byte].rgbBlue;
x++;
if (ptr > data_end)
ptr = data_end;
}
} }
else else
{ {
byte = *(buffer_ptr++); switch (byte2)
switch (byte)
{ {
case 0: case RLE_NEXT:
x = 0; x = 0;
y++; if (++y >= h) goto _bail;
ptr = im->data + ((h - y - 1) ptr = im->data + ((h - y - 1) * w) + x;
* w * sizeof(DATA32));
if (ptr > data_end)
ptr = data_end;
break; break;
case 1: case RLE_END:
g = 0; goto _bail;
break; case RLE_MOVE:
case 2: /* Need to read two bytes */
x += *(buffer_ptr++); if (buffer_ptr >= buffer_end_minus_1) goto _bail;
y += *(buffer_ptr++); x += buffer_ptr[0];
ptr = im->data + ((h - y - 1) y += buffer_ptr[1];
* w * buffer_ptr += 2;
sizeof(DATA32)) + /* Check for correct coordinates */
(x * sizeof(DATA32)); if (x >= w) goto _bail;
if (ptr > data_end) if (y >= h) goto _bail;
ptr = data_end; ptr = im->data + ((h - y - 1) * w) + x;
break; break;
default: default:
l = byte; l = byte2;
if (x + l > w) goto _bail;
if (buffer_ptr + l > buffer_end) goto _bail;
for (j = 0; j < l; j++) for (j = 0; j < l; j++)
{ {
byte = *(buffer_ptr++); byte = *(buffer_ptr++);
if (x >= w)
{
buffer_ptr += l - j;
break;
}
*ptr++ = 0xff000000 | *ptr++ = 0xff000000 |
(rgbQuads[byte].rgbRed << 16) | (rgbQuads[byte].rgbRed << 16) |
(rgbQuads[byte].rgbGreen << 8) | (rgbQuads[byte].rgbGreen << 8) |
rgbQuads[byte].rgbBlue; rgbQuads[byte].rgbBlue;
x++;
if (ptr > data_end)
ptr = data_end;
} }
x += l;
if (l & 1) if (l & 1)
buffer_ptr++; buffer_ptr++;
break; break;
@ -639,17 +654,30 @@ load(ImlibImage * im, ImlibProgressFunction progress,
} }
else if (bitcount == 16) else if (bitcount == 16)
{ {
/* 21.3.2006 - Need to check for buffer_ptr + 1 < buffer_end */
unsigned char *buffer_end_minus_1 = buffer_end - 1;
skip = (((w * 16 + 31) / 32) * 4) - (w * 2); skip = (((w * 16 + 31) / 32) * 4) - (w * 2);
for (y = 0; y < h; y++) for (y = 0; y < h; y++)
{ {
for (x = 0; x < w && buffer_ptr < buffer_end; x++) for (x = 0; x < w && buffer_ptr < buffer_end_minus_1; x++)
{ {
r = ((unsigned short)(*buffer_ptr) & rmask) >> rshift; /*
g = ((unsigned short)(*buffer_ptr) & gmask) >> gshift; * THIS WAS OLD CODE
b = ((unsigned short)(*(buffer_ptr)) & bmask) >> *
bshift; * r = ((unsigned short)(*buffer_ptr) & rmask) >> rshift;
*ptr++ = 0xff000000 | (r << 16) | (g << 8) | b; * g = ((unsigned short)(*buffer_ptr) & gmask) >> gshift;
buffer_ptr += 2; * b = ((unsigned short)(*(buffer_ptr++)) & bmask) >>
* bshift;
* *ptr++ = 0xff000000 | (r << 16) | (g << 8) | b;
*/
/* TODO: I don't know if [rgb]shift are calculated correctly, because we
* 16 bit depth losses some values (bits).
*/
unsigned short pix = *(unsigned short *)buffer_ptr;
*ptr++ = 0xff000000 | (((pix & rmask) >> rshift) << 16) |
(((pix & gmask) >> gshift) << 8) |
(((pix & bmask) >> bshift) ) ;
buffer_ptr += 2;
} }
ptr -= w * 2; ptr -= w * 2;
buffer_ptr += skip; buffer_ptr += skip;
@ -678,10 +706,12 @@ load(ImlibImage * im, ImlibProgressFunction progress,
} }
else if (bitcount == 24) else if (bitcount == 24)
{ {
/* 21.3.2006 - Fix: need to check for buffer_ptr + 2 < buffer_end */
unsigned char *buffer_end_minus_2 = buffer_end - 2;
skip = (4 - ((w * 3) % 4)) & 3; skip = (4 - ((w * 3) % 4)) & 3;
for (y = 0; y < h; y++) for (y = 0; y < h; y++)
{ {
for (x = 0; x < w && buffer_ptr < buffer_end; x++) for (x = 0; x < w && buffer_ptr < buffer_end_minus_2; x++)
{ {
b = *(buffer_ptr++); b = *(buffer_ptr++);
g = *(buffer_ptr++); g = *(buffer_ptr++);
@ -715,16 +745,30 @@ load(ImlibImage * im, ImlibProgressFunction progress,
} }
else if (bitcount == 32) else if (bitcount == 32)
{ {
/* 21.3.2006 - Need to check buffer_ptr + 3 < buffer_end */
unsigned char *buffer_end_minus_3 = buffer_end_minus_3;
skip = (((w * 32 + 31) / 32) * 4) - (w * 4); skip = (((w * 32 + 31) / 32) * 4) - (w * 4);
for (y = 0; y < h; y++) for (y = 0; y < h; y++)
{ {
for (x = 0; x < w && buffer_ptr < buffer_end; x++) for (x = 0; x < w && buffer_ptr < buffer_end_minus_3; x++)
{ {
r = ((unsigned long)(*buffer_ptr) & rmask) >> rshift; /*
g = ((unsigned long)(*buffer_ptr) & gmask) >> gshift; * THIS WAS OLD CODE: I don't understand it and it's invalid.
b = ((unsigned long)(*buffer_ptr) & bmask) >> bshift; *
*ptr++ = 0xff000000 | (r << 16) | (g << 8) | b; * r = ((unsigned long)(*buffer_ptr) & rmask) >> rshift;
buffer_ptr+=4; * g = ((unsigned long)(*buffer_ptr) & gmask) >> gshift;
* b = ((unsigned long)(*buffer_ptr) & bmask) >> bshift;
* *ptr++ = 0xff000000 | (r << 16) | (g << 8) | b;
* r = *(buffer_ptr++);
* r = *(buffer_ptr++);
*/
/* TODO: What about alpha channel...Is used? */
DATA32 pix = *(unsigned int *)buffer_ptr;
*ptr++ = 0xff000000 | (((pix & rmask) >> rshift) << 16) |
(((pix & gmask) >> gshift) << 8) |
(((pix & bmask) >> bshift) ) ;
buffer_ptr += 4;
} }
ptr -= w * 2; ptr -= w * 2;
buffer_ptr += skip; buffer_ptr += skip;
@ -751,6 +795,7 @@ load(ImlibImage * im, ImlibProgressFunction progress,
} }
} }
} }
_bail:
free(buffer); free(buffer);
} }
return 1; return 1;