From 42ff1aa86ccb24e35c058385ef61c737024ea3fc Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Sun, 23 Jul 2017 11:33:05 +0200 Subject: [PATCH] Fix overflow checks in mbfl_memory_device Also prune out some duplicate code and use strlen() and memcpy() instead of ad-hoc reimplementations. Remove multiplications by sizeof(unsigned char), which wrongly imply that this can be anything but 1. --- ext/mbstring/libmbfl/mbfl/mbfl_defs.h | 4 + .../libmbfl/mbfl/mbfl_memory_device.c | 133 ++++++------------ 2 files changed, 45 insertions(+), 92 deletions(-) diff --git a/ext/mbstring/libmbfl/mbfl/mbfl_defs.h b/ext/mbstring/libmbfl/mbfl/mbfl_defs.h index 753724d7be..8b18e5e0c4 100644 --- a/ext/mbstring/libmbfl/mbfl/mbfl_defs.h +++ b/ext/mbstring/libmbfl/mbfl/mbfl_defs.h @@ -39,6 +39,10 @@ #endif #endif +#ifndef SIZE_MAX +#define SIZE_MAX ((size_t)~0) +#endif + #ifdef WIN32 #ifdef MBFL_DLL_EXPORT #define MBFLAPI __declspec(dllexport) diff --git a/ext/mbstring/libmbfl/mbfl/mbfl_memory_device.c b/ext/mbstring/libmbfl/mbfl/mbfl_memory_device.c index 176a752288..bfa477fa06 100644 --- a/ext/mbstring/libmbfl/mbfl/mbfl_memory_device.c +++ b/ext/mbstring/libmbfl/mbfl/mbfl_memory_device.c @@ -36,6 +36,7 @@ #include #endif +#include #include "mbfl_allocators.h" #include "mbfl_string.h" #include "mbfl_memory_device.h" @@ -48,9 +49,9 @@ mbfl_memory_device_init(mbfl_memory_device *device, size_t initsz, size_t allocs { if (device) { device->length = 0; - device->buffer = (unsigned char *)0; + device->buffer = NULL; if (initsz > 0) { - device->buffer = (unsigned char *)mbfl_malloc(initsz*sizeof(unsigned char)); + device->buffer = (unsigned char *)mbfl_malloc(initsz); if (device->buffer != NULL) { device->length = initsz; } @@ -71,7 +72,7 @@ mbfl_memory_device_realloc(mbfl_memory_device *device, size_t initsz, size_t all if (device) { if (initsz > device->length) { - tmp = (unsigned char *)mbfl_realloc((void *)device->buffer, initsz*sizeof(unsigned char)); + tmp = (unsigned char *)mbfl_realloc((void *)device->buffer, initsz); if (tmp != NULL) { device->buffer = tmp; device->length = initsz; @@ -92,7 +93,7 @@ mbfl_memory_device_clear(mbfl_memory_device *device) if (device->buffer) { mbfl_free(device->buffer); } - device->buffer = (unsigned char *)0; + device->buffer = NULL; device->length = 0; device->pos = 0; } @@ -121,7 +122,7 @@ mbfl_memory_device_result(mbfl_memory_device *device, mbfl_string *result) result->len = device->pos; mbfl_memory_device_output('\0', device); result->val = device->buffer; - device->buffer = (unsigned char *)0; + device->buffer = NULL; device->length = 0; device->pos= 0; if (result->val == NULL) { @@ -145,12 +146,13 @@ mbfl_memory_device_output(int c, void *data) size_t newlen; unsigned char *tmp; - newlen = device->length + device->allocsz; - if (newlen <= 0) { + if (device->length > SIZE_MAX - device->allocsz) { /* overflow */ return -1; } - tmp = (unsigned char *)mbfl_realloc((void *)device->buffer, newlen*sizeof(unsigned char)); + + newlen = device->length + device->allocsz; + tmp = (unsigned char *)mbfl_realloc((void *)device->buffer, newlen); if (tmp == NULL) { return -1; } @@ -167,17 +169,18 @@ mbfl_memory_device_output2(int c, void *data) { mbfl_memory_device *device = (mbfl_memory_device *)data; - if ((device->pos + 2) >= device->length) { + if (2 > device->length - device->pos) { /* reallocate buffer */ size_t newlen; unsigned char *tmp; - newlen = device->length + device->allocsz; - if (newlen <= 0) { + if (device->length > SIZE_MAX - device->allocsz) { /* overflow */ return -1; } - tmp = (unsigned char *)mbfl_realloc((void *)device->buffer, newlen*sizeof(unsigned char)); + + newlen = device->length + device->allocsz; + tmp = (unsigned char *)mbfl_realloc((void *)device->buffer, newlen); if (tmp == NULL) { return -1; } @@ -196,17 +199,18 @@ mbfl_memory_device_output4(int c, void* data) { mbfl_memory_device *device = (mbfl_memory_device *)data; - if ((device->pos + 4) >= device->length) { + if (4 > device->length - device->pos) { /* reallocate buffer */ size_t newlen; unsigned char *tmp; - newlen = device->length + device->allocsz; - if (newlen <= 0) { + if (device->length > SIZE_MAX - device->allocsz) { /* overflow */ return -1; } - tmp = (unsigned char *)mbfl_realloc((void *)device->buffer, newlen*sizeof(unsigned char)); + + newlen = device->length + device->allocsz; + tmp = (unsigned char *)mbfl_realloc((void *)device->buffer, newlen); if (tmp == NULL) { return -1; } @@ -225,42 +229,7 @@ mbfl_memory_device_output4(int c, void* data) int mbfl_memory_device_strcat(mbfl_memory_device *device, const char *psrc) { - size_t len; - unsigned char *w; - const unsigned char *p; - - len = 0; - p = (const unsigned char*)psrc; - while (*p) { - p++; - len++; - } - - if ((device->pos + len) >= device->length) { - /* reallocate buffer */ - size_t newlen = device->length + (len + MBFL_MEMORY_DEVICE_ALLOC_SIZE)*sizeof(unsigned char); - unsigned char *tmp; - if (newlen <= 0) { - /* overflow */ - return -1; - } - tmp = (unsigned char *)mbfl_realloc((void *)device->buffer, newlen*sizeof(unsigned char)); - if (tmp == NULL) { - return -1; - } - device->length = newlen; - device->buffer = tmp; - } - - p = (const unsigned char*)psrc; - w = &device->buffer[device->pos]; - device->pos += len; - while (len > 0) { - *w++ = *p++; - len--; - } - - return 0; + return mbfl_memory_device_strncat(device, psrc, strlen(psrc)); } int @@ -268,28 +237,30 @@ mbfl_memory_device_strncat(mbfl_memory_device *device, const char *psrc, size_t { unsigned char *w; - if ((device->pos + len) >= device->length) { + if (len > device->length - device->pos) { /* reallocate buffer */ - size_t newlen = device->length + len + MBFL_MEMORY_DEVICE_ALLOC_SIZE; + size_t newlen; unsigned char *tmp; - if (newlen <= 0) { + + if (len > SIZE_MAX - MBFL_MEMORY_DEVICE_ALLOC_SIZE + || device->length > SIZE_MAX - (len + MBFL_MEMORY_DEVICE_ALLOC_SIZE)) { /* overflow */ return -1; } - tmp = (unsigned char *)mbfl_realloc((void *)device->buffer, newlen*sizeof(unsigned char)); + + newlen = device->length + len + MBFL_MEMORY_DEVICE_ALLOC_SIZE; + tmp = (unsigned char *)mbfl_realloc((void *)device->buffer, newlen); if (tmp == NULL) { return -1; } + device->length = newlen; device->buffer = tmp; } w = &device->buffer[device->pos]; + memcpy(w, psrc, len); device->pos += len; - while (len > 0) { - *w++ = *psrc++; - len--; - } return 0; } @@ -297,42 +268,14 @@ mbfl_memory_device_strncat(mbfl_memory_device *device, const char *psrc, size_t int mbfl_memory_device_devcat(mbfl_memory_device *dest, mbfl_memory_device *src) { - size_t n; - unsigned char *p, *w; - - if ((dest->pos + src->pos) >= dest->length) { - /* reallocate buffer */ - size_t newlen = dest->length + src->pos + MBFL_MEMORY_DEVICE_ALLOC_SIZE; - unsigned char *tmp; - if (newlen <= 0) { - /* overflow */ - return -1; - } - tmp = (unsigned char *)mbfl_realloc((void *)dest->buffer, newlen*sizeof(unsigned char)); - if (tmp == NULL) { - return -1; - } - dest->length = newlen; - dest->buffer = tmp; - } - - p = src->buffer; - w = &dest->buffer[dest->pos]; - n = src->pos; - dest->pos += n; - while (n > 0) { - *w++ = *p++; - n--; - } - - return 0; + return mbfl_memory_device_strncat(dest, (const char *) src->buffer, src->pos); } void mbfl_wchar_device_init(mbfl_wchar_device *device) { if (device) { - device->buffer = (unsigned int *)0; + device->buffer = NULL; device->length = 0; device->pos= 0; device->allocsz = MBFL_MEMORY_DEVICE_ALLOC_SIZE; @@ -346,7 +289,7 @@ mbfl_wchar_device_clear(mbfl_wchar_device *device) if (device->buffer) { mbfl_free(device->buffer); } - device->buffer = (unsigned int*)0; + device->buffer = NULL; device->length = 0; device->pos = 0; } @@ -362,11 +305,17 @@ mbfl_wchar_device_output(int c, void *data) size_t newlen; unsigned int *tmp; + if (device->length > SIZE_MAX - device->allocsz) { + /* overflow */ + return -1; + } + newlen = device->length + device->allocsz; - if (newlen <= 0) { + if (newlen > SIZE_MAX / sizeof(int)) { /* overflow */ return -1; } + tmp = (unsigned int *)mbfl_realloc((void *)device->buffer, newlen*sizeof(int)); if (tmp == NULL) { return -1; -- 2.40.0