From 81609be0524a4f38e1187fd6a0a0c8bcd4e17d4c Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Fri, 28 Jun 2019 19:38:45 +0100 Subject: [PATCH] Check for overflow in the addition in snew_plus(). We were carefully checking for overflow inside safemalloc() before multiplying together the two factors of the desired allocation size. But snew_plus() did an addition _outside_ safemalloc, without the same guard. Now that addition also happens inside safemalloc. --- memory.c | 33 +++++++++++++++++++++------------ puttymem.h | 10 +++++----- 2 files changed, 26 insertions(+), 17 deletions(-) diff --git a/memory.c b/memory.c index b6fb325d..5d340253 100644 --- a/memory.c +++ b/memory.c @@ -10,26 +10,35 @@ #include "puttymem.h" #include "misc.h" -void *safemalloc(size_t n, size_t size) +void *safemalloc(size_t factor1, size_t factor2, size_t addend) { - void *p; + if (factor1 > SIZE_MAX / factor2) + goto fail; + size_t product = factor1 * factor2; - if (n > INT_MAX / size) { - p = NULL; - } else { - size *= n; - if (size == 0) size = 1; + if (addend > SIZE_MAX) + goto fail; + if (product > SIZE_MAX - addend) + goto fail; + size_t size = product + addend; + + if (size == 0) + size = 1; + + void *p; #ifdef MINEFIELD - p = minefield_c_malloc(size); + p = minefield_c_malloc(size); #else - p = malloc(size); + p = malloc(size); #endif - } if (!p) - out_of_memory(); + goto fail; return p; + + fail: + out_of_memory(); } void *saferealloc(void *ptr, size_t n, size_t size) @@ -111,7 +120,7 @@ void *safegrowarray(void *ptr, size_t *allocated, size_t eltsize, size_t newsize = oldsize + increment; void *toret; if (secret) { - toret = safemalloc(newsize, eltsize); + toret = safemalloc(newsize, eltsize, 0); memcpy(toret, ptr, oldsize * eltsize); smemclr(ptr, oldsize * eltsize); sfree(ptr); diff --git a/puttymem.h b/puttymem.h index 85184a14..ac1bd681 100644 --- a/puttymem.h +++ b/puttymem.h @@ -10,13 +10,13 @@ #include "defs.h" -#define smalloc(z) safemalloc(z,1) +#define smalloc(z) safemalloc(z,1,0) #define snmalloc safemalloc #define srealloc(y,z) saferealloc(y,z,1) #define snrealloc saferealloc #define sfree safefree -void *safemalloc(size_t, size_t); +void *safemalloc(size_t factor1, size_t factor2, size_t addend); void *saferealloc(void *, size_t, size_t); void safefree(void *); @@ -28,8 +28,8 @@ void safefree(void *); * TYPECHECK to verify that the _input_ pointer is a pointer to the * correct type. */ -#define snew(type) ((type *)snmalloc(1, sizeof(type))) -#define snewn(n, type) ((type *)snmalloc((n), sizeof(type))) +#define snew(type) ((type *)snmalloc(1, sizeof(type), 0)) +#define snewn(n, type) ((type *)snmalloc((n), sizeof(type), 0)) #define sresize(ptr, n, type) TYPECHECK((type *)0 == (ptr), \ ((type *)snrealloc((ptr), (n), sizeof(type)))) @@ -45,7 +45,7 @@ void safefree(void *); * result to void *, so you can assign it straight to wherever you * wanted it. */ -#define snew_plus(type, extra) ((type *)snmalloc(1, sizeof(type) + (extra))) +#define snew_plus(type, extra) ((type *)snmalloc(1, sizeof(type), (extra))) #define snew_plus_get_aux(ptr) ((void *)((ptr) + 1)) /*