Hi again,
here is slightly better patch.
Cheers,
Tomasz
From: Tomasz Buchert <tomasz.buch...@inria.fr>
Date: Sun, 1 Mar 2015 20:27:29 +0100
Subject: Fix multiple integer overflows.
Dimensions of a BMP file are signed, 4-byte integers. Therefore
the size of the image may be bigger than range of (int). This is fixed
in bitmap.h, by casting all offsets to long int (and fixing
another possible overflow in bm_new).
In bitmap_io.c we make sure that width and height of the image
are non-negative and in (int) range, because other functions require it.
---
src/bitmap.h | 20 +++++++++++++-------
src/bitmap_io.c | 27 +++++++++++++++++++++++++--
2 files changed, 38 insertions(+), 9 deletions(-)
diff --git a/src/bitmap.h b/src/bitmap.h
index 1ce13d6..878aa9a 100644
--- a/src/bitmap.h
+++ b/src/bitmap.h
@@ -27,7 +27,8 @@
/* macros for accessing pixel at index (x,y). U* macros omit the
bounds check. */
-#define bm_scanline(bm, y) ((bm)->map + (y)*(bm)->dy)
+#define bm_allocsize(bm) ((long int)(bm)->dy * (long int)(bm)->h)
+#define bm_scanline(bm, y) ((bm)->map + ((long int)(y))*(long int)(bm)->dy)
#define bm_index(bm, x, y) (&bm_scanline(bm, y)[(x)/BM_WORDBITS])
#define bm_mask(x) (BM_HIBIT >> ((x) & (BM_WORDBITS-1)))
#define bm_range(x, a) ((int)(x) >= 0 && (int)(x) < (a))
@@ -54,7 +55,12 @@ static inline void bm_free(potrace_bitmap_t *bm) {
/* return new un-initialized bitmap. NULL with errno on error */
static inline potrace_bitmap_t *bm_new(int w, int h) {
potrace_bitmap_t *bm;
- int dy = (w + BM_WORDBITS - 1) / BM_WORDBITS;
+ int dy;
+
+ if (w % BM_WORDBITS == 0)
+ dy = w / BM_WORDBITS;
+ else
+ dy = (w / BM_WORDBITS) + 1;
bm = (potrace_bitmap_t *) malloc(sizeof(potrace_bitmap_t));
if (!bm) {
@@ -63,7 +69,7 @@ static inline potrace_bitmap_t *bm_new(int w, int h) {
bm->w = w;
bm->h = h;
bm->dy = dy;
- bm->map = (potrace_word *) malloc(dy * h * BM_WORDSIZE);
+ bm->map = (potrace_word *) malloc(bm_allocsize(bm) * BM_WORDSIZE);
if (!bm->map) {
free(bm);
return NULL;
@@ -73,7 +79,7 @@ static inline potrace_bitmap_t *bm_new(int w, int h) {
/* clear the given bitmap. Set all bits to c. */
static inline void bm_clear(potrace_bitmap_t *bm, int c) {
- memset(bm->map, c ? -1 : 0, bm->dy * bm->h * BM_WORDSIZE);
+ memset(bm->map, c ? -1 : 0, bm_allocsize(bm) * BM_WORDSIZE);
}
/* duplicate the given bitmap. Return NULL on error with errno set. */
@@ -82,14 +88,14 @@ static inline potrace_bitmap_t *bm_dup(const potrace_bitmap_t *bm) {
if (!bm1) {
return NULL;
}
- memcpy(bm1->map, bm->map, bm->dy * bm->h * BM_WORDSIZE);
+ memcpy(bm1->map, bm->map, bm_allocsize(bm) * BM_WORDSIZE);
return bm1;
}
/* invert the given bitmap. */
static inline void bm_invert(potrace_bitmap_t *bm) {
- int i;
- for (i = 0; i < bm->dy * bm->h; i++) {
+ long int i;
+ for (i = 0; i < bm_allocsize(bm); i++) {
bm->map[i] ^= BM_ALLBITS;
}
}
diff --git a/src/bitmap_io.c b/src/bitmap_io.c
index 6cfecb1..3635ec3 100644
--- a/src/bitmap_io.c
+++ b/src/bitmap_io.c
@@ -381,6 +381,16 @@ static int bmp_readint(FILE *f, int n, unsigned int *p) {
return 0;
}
+/* converts unsigned to signed using 32 bits */
+static int unsigned_to_signed32(unsigned int x) {
+ unsigned int sign = 0x80000000U;
+ unsigned int mask = 0xFFFFFFFFU;
+ if (sign & x)
+ return -(int)(x ^ mask) - 1;
+ else
+ return (int)x;
+}
+
/* reset padding boundary */
static void bmp_pad_reset(void) {
bmp_count = 0;
@@ -478,12 +488,25 @@ static int bm_readbody_bmp(FILE *f, double threshold, potrace_bitmap_t **bmp) {
TRY(bmp_readint(f, 4, &bmpinfo.BlueMask));
TRY(bmp_readint(f, 4, &bmpinfo.AlphaMask));
}
- if ((signed int)bmpinfo.h < 0) {
- bmpinfo.h = -bmpinfo.h;
+ int maxdim = 0x7ffffffe; /* 2^31 - 2 */
+ int sign_h = unsigned_to_signed32(bmpinfo.h);
+ int sign_w = unsigned_to_signed32(bmpinfo.w);
+ if (sign_w < 0 || sign_w > maxdim) {
+ bm_read_error = "incorrect bmp width";
+ goto format_error;
+ }
+ if (sign_h < -maxdim || sign_h > maxdim) {
+ bm_read_error = "incorrect bmp height";
+ goto format_error;
+ }
+ if (sign_h < 0) {
+ bmpinfo.h = (unsigned int)(-sign_h);
bmpinfo.topdown = 1;
} else {
bmpinfo.topdown = 0;
}
+ /* now we know that bmpinfo.{w,h} are non-negative ints
+ that fit in int type (cf. bm_new)) */
} else if (bmpinfo.InfoSize == 12) {
/* old OS/2 format */
bmpinfo.ctbits = 24; /* sample size in color table */
