Here's the patch that I am planning to apply upstream. Please comment
if you see anything wrong with it.
While the general idea is similar to Tomasz's patch, I've solved the
details a bit differently.
* I prefer to use ssize_t instead of unsigned long long int for memory
manipulations. Since size_t is the type used by malloc, memcpy, etc,
it is big enough to hold the relevant values. The reason I use a
signed rather than unsigned type is that the "dy" field in the
potrace_bitmap_s structure may be positive or negative, depending on
whether the bitmap is stored top-to-bottom or bottom-to-top. Potrace
itself always uses a positive dy, but other applications that link
against the Potrace library may use their own convention. Tomasz's
patch used an unsigned type which would break applications that use
a negative dy.
The code now checks that the bitmap dimensions are indeed such that
all relevant values fit within ssize_t. A remaining assumption is
that ssize_t is at least as big as int, which I think is guaranteed.
* I prefer to use calloc instead of safe_malloc. Calloc is appropriate
whenever the memory to be allocated is a number of copies of items
of a given size. Unlike malloc(x*y), calloc(x, y) actually checks
that x*y does not overflow. (I checked the glibc source code for
calloc to be sure that such a check is actually performed). In the
few cases where the argument of malloc is calculated differently
(say as a product of three numbers), I have added an explicit
overflow check. This is safer, in my opinion, than safe_malloc(x*y);
in particular, there is no difference bewteen safe_malloc and
ordinary malloc when size_t = unsigned long long int.
* I also fixed analogous issues in Mkbitmap and throughout the rest of
the code.
I'll post an updated upstream package in a day or two unless there's
feedback requiring additional changes.
Thanks, -- Peter
diff -u -r potrace-1.11/src/backend_eps.c potrace-1.11-patched/src/backend_eps.c
--- potrace-1.11/src/backend_eps.c 2013-02-20 00:51:44.000000000 +0100
+++ potrace-1.11-patched/src/backend_eps.c 2015-03-20 23:57:39.703432480
+0100
@@ -26,8 +26,8 @@
#include "config.h"
#endif
-#define SAFE_MALLOC(var, n, typ) \
- if ((var = (typ *)malloc((n)*sizeof(typ))) == NULL) goto malloc_error
+#define SAFE_CALLOC(var, n, typ) \
+ if ((var = (typ *)calloc(n, sizeof(typ))) == NULL) goto calloc_error
typedef int color_t;
@@ -232,10 +232,10 @@
double M;
int m = curve->n;
- SAFE_MALLOC(bq, m, long int);
- SAFE_MALLOC(aq, m, long int);
- SAFE_MALLOC(v, m, point_t);
- SAFE_MALLOC(q, m, dpoint_t);
+ SAFE_CALLOC(bq, m, long int);
+ SAFE_CALLOC(aq, m, long int);
+ SAFE_CALLOC(v, m, point_t);
+ SAFE_CALLOC(q, m, dpoint_t);
/* quantize vertices */
for (i=0; i<m; i++) {
@@ -295,7 +295,7 @@
free(q);
return 0;
- malloc_error:
+ calloc_error:
free(bq);
free(aq);
free(v);
diff -u -r potrace-1.11/src/bitmap.h potrace-1.11-patched/src/bitmap.h
--- potrace-1.11/src/bitmap.h 2013-02-20 00:51:44.000000000 +0100
+++ potrace-1.11-patched/src/bitmap.h 2015-03-20 23:57:39.704432477 +0100
@@ -7,6 +7,7 @@
#include <string.h>
#include <stdlib.h>
+#include <errno.h>
/* The bitmap type is defined in potracelib.h */
#include "potracelib.h"
@@ -27,7 +28,7 @@
/* 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_scanline(bm, y) ((bm)->map + (y)*(ssize_t)(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))
@@ -51,10 +52,18 @@
free(bm);
}
-/* return new un-initialized bitmap. NULL with errno on error */
+/* return new un-initialized bitmap. NULL with errno on error.
+ Assumes w, h >= 0. */
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 = w == 0 ? 0 : (w - 1) / BM_WORDBITS + 1;
+ ssize_t size = (ssize_t)dy * (ssize_t)h * (ssize_t)BM_WORDSIZE;
+
+ /* check for overflow error */
+ if (size < 0 || size / h / dy != BM_WORDSIZE) {
+ errno = ENOMEM;
+ return NULL;
+ }
bm = (potrace_bitmap_t *) malloc(sizeof(potrace_bitmap_t));
if (!bm) {
@@ -63,7 +72,7 @@
bm->w = w;
bm->h = h;
bm->dy = dy;
- bm->map = (potrace_word *) malloc(dy * h * BM_WORDSIZE);
+ bm->map = (potrace_word *) malloc(size);
if (!bm->map) {
free(bm);
return NULL;
@@ -73,23 +82,29 @@
/* 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);
+ /* Note: if the bitmap was created with bm_new, then it is
+ guaranteed that size will fit into the ssize_t type. */
+ ssize_t size = (ssize_t)bm->dy * (ssize_t)bm->h * (ssize_t)BM_WORDSIZE;
+ memset(bm->map, c ? -1 : 0, size);
}
/* duplicate the given bitmap. Return NULL on error with errno set. */
static inline potrace_bitmap_t *bm_dup(const potrace_bitmap_t *bm) {
potrace_bitmap_t *bm1 = bm_new(bm->w, bm->h);
+ ssize_t size = (ssize_t)bm->dy * (ssize_t)bm->h * (ssize_t)BM_WORDSIZE;
if (!bm1) {
return NULL;
}
- memcpy(bm1->map, bm->map, bm->dy * bm->h * BM_WORDSIZE);
+ memcpy(bm1->map, bm->map, size);
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++) {
+ ssize_t i;
+ ssize_t size = (ssize_t)bm->dy * (ssize_t)bm->h;
+
+ for (i = 0; i < size; i++) {
bm->map[i] ^= BM_ALLBITS;
}
}
diff -u -r potrace-1.11/src/bitmap_io.c potrace-1.11-patched/src/bitmap_io.c
--- potrace-1.11/src/bitmap_io.c 2013-02-20 00:51:44.000000000 +0100
+++ potrace-1.11-patched/src/bitmap_io.c 2015-03-20 23:57:39.706432473
+0100
@@ -478,12 +478,18 @@
TRY(bmp_readint(f, 4, &bmpinfo.BlueMask));
TRY(bmp_readint(f, 4, &bmpinfo.AlphaMask));
}
- if ((signed int)bmpinfo.h < 0) {
- bmpinfo.h = -bmpinfo.h;
+ if (bmpinfo.w > 0x7fffffff) {
+ goto format_error;
+ }
+ if (bmpinfo.h > 0x7fffffff) {
+ bmpinfo.h = (-bmpinfo.h) & 0xffffffff;
bmpinfo.topdown = 1;
} else {
bmpinfo.topdown = 0;
}
+ if (bmpinfo.h > 0x7fffffff) {
+ goto format_error;
+ }
} else if (bmpinfo.InfoSize == 12) {
/* old OS/2 format */
bmpinfo.ctbits = 24; /* sample size in color table */
@@ -517,7 +523,7 @@
/* color table, present only if bmpinfo.bits <= 8. */
if (bmpinfo.bits <= 8) {
- coltable = (int *) malloc(bmpinfo.ncolors * sizeof(int));
+ coltable = (int *) calloc(bmpinfo.ncolors, sizeof(int));
if (!coltable) {
goto std_error;
}
diff -u -r potrace-1.11/src/curve.c potrace-1.11-patched/src/curve.c
--- potrace-1.11/src/curve.c 2013-02-20 00:51:44.000000000 +0100
+++ potrace-1.11-patched/src/curve.c 2015-03-20 23:57:39.707432471 +0100
@@ -12,8 +12,8 @@
#include "lists.h"
#include "curve.h"
-#define SAFE_MALLOC(var, n, typ) \
- if ((var = (typ *)malloc((n)*sizeof(typ))) == NULL) goto malloc_error
+#define SAFE_CALLOC(var, n, typ) \
+ if ((var = (typ *)calloc(n, sizeof(typ))) == NULL) goto calloc_error
/* ---------------------------------------------------------------------- */
/* allocate and free path objects */
@@ -22,14 +22,14 @@
path_t *p = NULL;
privpath_t *priv = NULL;
- SAFE_MALLOC(p, 1, path_t);
+ SAFE_CALLOC(p, 1, path_t);
memset(p, 0, sizeof(path_t));
- SAFE_MALLOC(priv, 1, privpath_t);
+ SAFE_CALLOC(priv, 1, privpath_t);
memset(priv, 0, sizeof(privpath_t));
p->priv = priv;
return p;
- malloc_error:
+ calloc_error:
free(p);
free(priv);
return NULL;
@@ -81,15 +81,15 @@
int privcurve_init(privcurve_t *curve, int n) {
memset(curve, 0, sizeof(privcurve_t));
curve->n = n;
- SAFE_MALLOC(curve->tag, n, int);
- SAFE_MALLOC(curve->c, n, dpoint3_t);
- SAFE_MALLOC(curve->vertex, n, dpoint_t);
- SAFE_MALLOC(curve->alpha, n, double);
- SAFE_MALLOC(curve->alpha0, n, double);
- SAFE_MALLOC(curve->beta, n, double);
+ SAFE_CALLOC(curve->tag, n, int);
+ SAFE_CALLOC(curve->c, n, dpoint3_t);
+ SAFE_CALLOC(curve->vertex, n, dpoint_t);
+ SAFE_CALLOC(curve->alpha, n, double);
+ SAFE_CALLOC(curve->alpha0, n, double);
+ SAFE_CALLOC(curve->beta, n, double);
return 0;
- malloc_error:
+ calloc_error:
free(curve->tag);
free(curve->c);
free(curve->vertex);
diff -u -r potrace-1.11/src/greymap.c potrace-1.11-patched/src/greymap.c
--- potrace-1.11/src/greymap.c 2013-02-20 00:51:44.000000000 +0100
+++ potrace-1.11-patched/src/greymap.c 2015-03-20 23:57:39.709432467 +0100
@@ -9,6 +9,7 @@
#include <stdlib.h>
#include <string.h>
#include <math.h>
+#include <errno.h>
#include "greymap.h"
#include "bitops.h"
@@ -23,10 +24,17 @@
/* ---------------------------------------------------------------------- */
/* basic greymap routines */
-/* return new un-initialized greymap. NULL with errno on error */
-
+/* return new un-initialized greymap. NULL with errno on error.
+ Assumes w, h >= 0. */
greymap_t *gm_new(int w, int h) {
greymap_t *gm;
+ ssize_t size = (ssize_t)w * (ssize_t)h * (ssize_t)sizeof(signed short int);
+
+ /* check for overflow error */
+ if (size < 0 || size / w / h != sizeof(signed short int)) {
+ errno = ENOMEM;
+ return NULL;
+ }
gm = (greymap_t *) malloc(sizeof(greymap_t));
if (!gm) {
@@ -34,7 +42,7 @@
}
gm->w = w;
gm->h = h;
- gm->map = (signed short int *) malloc(w*h*sizeof(signed short int));
+ gm->map = (signed short int *) malloc(size);
if (!gm->map) {
free(gm);
return NULL;
@@ -537,12 +545,18 @@
TRY(bmp_readint(f, 4, &bmpinfo.BlueMask));
TRY(bmp_readint(f, 4, &bmpinfo.AlphaMask));
}
- if ((signed int)bmpinfo.h < 0) {
- bmpinfo.h = -bmpinfo.h;
+ if (bmpinfo.w > 0x7fffffff) {
+ goto format_error;
+ }
+ if (bmpinfo.h > 0x7fffffff) {
+ bmpinfo.h = (-bmpinfo.h) & 0xffffffff;
bmpinfo.topdown = 1;
} else {
bmpinfo.topdown = 0;
}
+ if (bmpinfo.h > 0x7fffffff) {
+ goto format_error;
+ }
} else if (bmpinfo.InfoSize == 12) {
/* old OS/2 format */
bmpinfo.ctbits = 24; /* sample size in color table */
@@ -576,7 +590,7 @@
/* color table, present only if bmpinfo.bits <= 8. */
if (bmpinfo.bits <= 8) {
- coltable = (int *) malloc(bmpinfo.ncolors * sizeof(int));
+ coltable = (int *) calloc(bmpinfo.ncolors, sizeof(int));
if (!coltable) {
goto std_error;
}
diff -u -r potrace-1.11/src/greymap.h potrace-1.11-patched/src/greymap.h
--- potrace-1.11/src/greymap.h 2013-02-20 00:51:44.000000000 +0100
+++ potrace-1.11-patched/src/greymap.h 2015-03-20 23:57:39.710432465 +0100
@@ -22,7 +22,7 @@
/* macros for accessing pixel at index (x,y). Note that the origin is
in the *lower* left corner. U* macros omit the bounds check. */
-#define gm_index(gm, x, y) (&(gm)->map[(x)+(y)*(gm)->w])
+#define gm_index(gm, x, y) (&(gm)->map[(x)+(y)*(ssize_t)(gm)->w])
#define gm_safe(gm, x, y) ((int)(x)>=0 && (int)(x)<(gm)->w && (int)(y)>=0 &&
(int)(y)<(gm)->h)
#define gm_bound(x, m) ((x)<0 ? 0 : (x)>=(m) ? (m)-1 : (x))
#define GM_UGET(gm, x, y) (*gm_index(gm, x, y))
diff -u -r potrace-1.11/src/mkbitmap.c potrace-1.11-patched/src/mkbitmap.c
--- potrace-1.11/src/mkbitmap.c 2013-02-20 00:51:44.000000000 +0100
+++ potrace-1.11-patched/src/mkbitmap.c 2015-03-20 23:57:39.712432461 +0100
@@ -22,8 +22,8 @@
#include "config.h"
#endif
-#define SAFE_MALLOC(var, n, typ) \
- if ((var = (typ *)malloc((n)*sizeof(typ))) == NULL) goto malloc_error
+#define SAFE_CALLOC(var, n, typ) \
+ if ((var = (typ *)calloc(n, sizeof(typ))) == NULL) goto calloc_error
/* structure to hold command line options */
struct info_s {
@@ -258,7 +258,7 @@
/* same as interpolate_linear, except use cubic interpolation (slower
and better). */
-/* we need this typedef so that the SAFE_MALLOC macro will work */
+/* we need this typedef so that the SAFE_CALLOC macro will work */
typedef double double4[4];
static void *interpolate_cubic(greymap_t *gm, int s, int bilevel, double c) {
@@ -272,8 +272,8 @@
greymap_t *gm_out = NULL;
potrace_bitmap_t *bm_out = NULL;
- SAFE_MALLOC(poly, s, double4);
- SAFE_MALLOC(window, s, double4);
+ SAFE_CALLOC(poly, s, double4);
+ SAFE_CALLOC(window, s, double4);
w = gm->w;
h = gm->h;
@@ -282,14 +282,14 @@
if (bilevel) {
bm_out = bm_new(w*s, h*s);
if (!bm_out) {
- goto malloc_error;
+ goto calloc_error;
}
bm_clear(bm_out, 0);
c1 = c * 255;
} else {
gm_out = gm_new(w*s, h*s);
if (!gm_out) {
- goto malloc_error;
+ goto calloc_error;
}
}
@@ -361,7 +361,7 @@
return (void *)gm_out;
}
- malloc_error:
+ calloc_error:
free(poly);
free(window);
return NULL;
diff -u -r potrace-1.11/src/potracelib_demo.c
potrace-1.11-patched/src/potracelib_demo.c
--- potrace-1.11/src/potracelib_demo.c 2013-02-20 00:51:44.000000000 +0100
+++ potrace-1.11-patched/src/potracelib_demo.c 2015-03-20 23:57:39.713432459
+0100
@@ -43,7 +43,7 @@
bm->w = w;
bm->h = h;
bm->dy = dy;
- bm->map = (potrace_word *) malloc(dy * h * BM_WORDSIZE);
+ bm->map = (potrace_word *) calloc(h, dy * BM_WORDSIZE);
if (!bm->map) {
free(bm);
return NULL;
diff -u -r potrace-1.11/src/render.c potrace-1.11-patched/src/render.c
--- potrace-1.11/src/render.c 2013-02-20 00:51:44.000000000 +0100
+++ potrace-1.11-patched/src/render.c 2015-03-20 23:57:39.714432457 +0100
@@ -52,7 +52,7 @@
}
memset(rm, 0, sizeof(render_t));
rm->gm = gm;
- rm->incrow_buf = (int *) malloc(gm->h * sizeof(int));
+ rm->incrow_buf = (int *) calloc(gm->h, sizeof(int));
if (!rm->incrow_buf) {
free(rm);
return NULL;
diff -u -r potrace-1.11/src/trace.c potrace-1.11-patched/src/trace.c
--- potrace-1.11/src/trace.c 2013-02-20 00:51:44.000000000 +0100
+++ potrace-1.11-patched/src/trace.c 2015-03-20 23:57:39.716432453 +0100
@@ -21,8 +21,8 @@
#define COS179 -0.999847695156 /* the cosine of 179 degrees */
/* ---------------------------------------------------------------------- */
-#define SAFE_MALLOC(var, n, typ) \
- if ((var = (typ *)malloc((n)*sizeof(typ))) == NULL) goto malloc_error
+#define SAFE_CALLOC(var, n, typ) \
+ if ((var = (typ *)calloc(n, sizeof(typ))) == NULL) goto calloc_error
/* ---------------------------------------------------------------------- */
/* auxiliary functions */
@@ -265,7 +265,7 @@
int i, x, y;
int n = pp->len;
- SAFE_MALLOC(pp->sums, pp->len+1, sums_t);
+ SAFE_CALLOC(pp->sums, pp->len+1, sums_t);
/* origin */
pp->x0 = pp->pt[0].x;
@@ -284,7 +284,7 @@
}
return 0;
- malloc_error:
+ calloc_error:
return 1;
}
@@ -331,8 +331,8 @@
point_t dk; /* direction of k-k1 */
int a, b, c, d;
- SAFE_MALLOC(pivk, n, int);
- SAFE_MALLOC(nc, n, int);
+ SAFE_CALLOC(pivk, n, int);
+ SAFE_CALLOC(nc, n, int);
/* initialize the nc data structure. Point from each point to the
furthest future point to which it is connected by a vertical or
@@ -349,7 +349,7 @@
nc[i] = k;
}
- SAFE_MALLOC(pp->lon, n, int);
+ SAFE_CALLOC(pp->lon, n, int);
/* determine pivot points: for each i, let pivk[i] be the furthest k
such that all j with i<j<k lie on a line connecting i,k. */
@@ -458,7 +458,7 @@
free(nc);
return 0;
- malloc_error:
+ calloc_error:
free(pivk);
free(nc);
return 1;
@@ -537,12 +537,12 @@
double best;
int c;
- SAFE_MALLOC(pen, n+1, double);
- SAFE_MALLOC(prev, n+1, int);
- SAFE_MALLOC(clip0, n, int);
- SAFE_MALLOC(clip1, n+1, int);
- SAFE_MALLOC(seg0, n+1, int);
- SAFE_MALLOC(seg1, n+1, int);
+ SAFE_CALLOC(pen, n+1, double);
+ SAFE_CALLOC(prev, n+1, int);
+ SAFE_CALLOC(clip0, n, int);
+ SAFE_CALLOC(clip1, n+1, int);
+ SAFE_CALLOC(seg0, n+1, int);
+ SAFE_CALLOC(seg1, n+1, int);
/* calculate clipped paths */
for (i=0; i<n; i++) {
@@ -604,7 +604,7 @@
}
pp->m = m;
- SAFE_MALLOC(pp->po, m, int);
+ SAFE_CALLOC(pp->po, m, int);
/* read off shortest path */
for (i=n, j=m-1; i>0; j--) {
@@ -620,7 +620,7 @@
free(seg1);
return 0;
- malloc_error:
+ calloc_error:
free(pen);
free(prev);
free(clip0);
@@ -655,13 +655,13 @@
dpoint_t s;
int r;
- SAFE_MALLOC(ctr, m, dpoint_t);
- SAFE_MALLOC(dir, m, dpoint_t);
- SAFE_MALLOC(q, m, quadform_t);
+ SAFE_CALLOC(ctr, m, dpoint_t);
+ SAFE_CALLOC(dir, m, dpoint_t);
+ SAFE_CALLOC(q, m, quadform_t);
r = privcurve_init(&pp->curve, m);
if (r) {
- goto malloc_error;
+ goto calloc_error;
}
/* calculate "optimal" point-slope representation for each line
@@ -827,7 +827,7 @@
free(q);
return 0;
- malloc_error:
+ calloc_error:
free(ctr);
free(dir);
free(q);
@@ -1075,12 +1075,12 @@
int *convc = NULL; /* conv[m]: pre-computed convexities */
double *areac = NULL; /* cumarea[m+1]: cache for fast area computation */
- SAFE_MALLOC(pt, m+1, int);
- SAFE_MALLOC(pen, m+1, double);
- SAFE_MALLOC(len, m+1, int);
- SAFE_MALLOC(opt, m+1, opti_t);
- SAFE_MALLOC(convc, m, int);
- SAFE_MALLOC(areac, m+1, double);
+ SAFE_CALLOC(pt, m+1, int);
+ SAFE_CALLOC(pen, m+1, double);
+ SAFE_CALLOC(len, m+1, int);
+ SAFE_CALLOC(opt, m+1, opti_t);
+ SAFE_CALLOC(convc, m, int);
+ SAFE_CALLOC(areac, m+1, double);
/* pre-calculate convexity: +1 = right turn, -1 = left turn, 0 = corner */
for (i=0; i<m; i++) {
@@ -1134,10 +1134,10 @@
om = len[m];
r = privcurve_init(&pp->ocurve, om);
if (r) {
- goto malloc_error;
+ goto calloc_error;
}
- SAFE_MALLOC(s, om, double);
- SAFE_MALLOC(t, om, double);
+ SAFE_CALLOC(s, om, double);
+ SAFE_CALLOC(t, om, double);
j = m;
for (i=om-1; i>=0; i--) {
@@ -1182,7 +1182,7 @@
free(areac);
return 0;
- malloc_error:
+ calloc_error:
free(pt);
free(pen);
free(len);
