Re: [PATCH 04/21] harden REALLOC_ARRAY and xcalloc against size_t overflow
From: René Scharfe <hidden>
Date: 2016-06-15 23:08:20
Am 19.02.2016 um 12:22 schrieb Jeff King:
REALLOC_ARRAY inherently involves a multiplication which can overflow size_t, resulting in a much smaller buffer than we think we've allocated. We can easily harden it by using st_mult() to check for overflow. Likewise, we can add ALLOC_ARRAY to do the same thing for xmalloc calls.
Good idea!
quoted hunk ↗ jump to hunk
xcalloc() should already be fine, because it takes the two factors separately, assuming the system calloc actually checks for overflow. However, before we even hit the system calloc(), we do our memory_limit_check, which involves a multiplication. Let's check for overflow ourselves so that this limit cannot be bypassed. Signed-off-by: Jeff King <redacted> --- git-compat-util.h | 3 ++- wrapper.c | 3 +++ 2 files changed, 5 insertions(+), 1 deletion(-)diff --git a/git-compat-util.h b/git-compat-util.h index 0c65033..55c073d 100644 --- a/git-compat-util.h +++ b/git-compat-util.h@@ -779,7 +779,8 @@ extern int odb_pack_keep(char *name, size_t namesz, const unsigned char *sha1); extern char *xgetcwd(void); extern FILE *fopen_for_writing(const char *path); -#define REALLOC_ARRAY(x, alloc) (x) = xrealloc((x), (alloc) * sizeof(*(x))) +#define ALLOC_ARRAY(x, alloc) (x) = xmalloc(st_mult((alloc), sizeof(*(x)))) +#define REALLOC_ARRAY(x, alloc) (x) = xrealloc((x), st_mult((alloc), sizeof(*(x))))
st_mult(x, y) calls unsigned_mult_overflows(x, y), which divides by x. This division can be done at compile time if x is a constant. This can be guaranteed for all users of the two macros above by reversing the arguments of st_mult(), so that sizeof comes first. Probably not a big win, but why not do it if it's that easy? Or perhaps a macro like this could help here and in other places which use st_mult with sizeof: #define SIZEOF_MULT(x, n) st_mult(sizeof(x), (n)) (I'd call it ARRAY_SIZE, but that name is already taken. :) René