From: Neal Norwitz Date: Mon, 28 Jul 2008 05:22:45 +0000 (+0000) Subject: Backport r65182. This change modified from using the unsigned max value X-Git-Tag: v2.4.6c1~14 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=46b3a79459934a09eaf00caf7e36b20717afc09f;p=thirdparty%2FPython%2Fcpython.git Backport r65182. This change modified from using the unsigned max value to the signed max value similar to 2.5 and trunk. Issue #2620: Overflow checking when allocating or reallocating memory was not always being done properly in some python types and extension modules. PyMem_MALLOC, PyMem_REALLOC, PyMem_NEW and PyMem_RESIZE have all been updated to perform better checks and places in the code that would previously leak memory on the error path when such an allocation failed have been fixed. --- diff --git a/Include/pymem.h b/Include/pymem.h index 0e18f03d0126..a3eb0955a724 100644 --- a/Include/pymem.h +++ b/Include/pymem.h @@ -66,8 +66,12 @@ PyAPI_FUNC(void) PyMem_Free(void *); for malloc(0), which would be treated as an error. Some platforms would return a pointer with no memory behind it, which would break pymalloc. To solve these problems, allocate an extra byte. */ -#define PyMem_MALLOC(n) malloc((n) ? (n) : 1) -#define PyMem_REALLOC(p, n) realloc((p), (n) ? (n) : 1) +/* Returns NULL to indicate error if a negative size or size larger than + Py_ssize_t can represent is supplied. Helps prevents security holes. */ +#define PyMem_MALLOC(n) (((n) < 0 || (n) > INT_MAX) ? NULL \ + : malloc((n) ? (n) : 1)) +#define PyMem_REALLOC(p, n) (((n) < 0 || (n) > INT_MAX) ? NULL \ + : realloc((p), (n) ? (n) : 1)) #endif /* PYMALLOC_DEBUG */ @@ -80,24 +84,31 @@ PyAPI_FUNC(void) PyMem_Free(void *); * Type-oriented memory interface * ============================== * - * These are carried along for historical reasons. There's rarely a good - * reason to use them anymore (you can just as easily do the multiply and - * cast yourself). + * Allocate memory for n objects of the given type. Returns a new pointer + * or NULL if the request was too large or memory allocation failed. Use + * these macros rather than doing the multiplication yourself so that proper + * overflow checking is always done. */ #define PyMem_New(type, n) \ - ( assert((n) <= PY_SIZE_MAX / sizeof(type)) , \ + ( ((n) > INT_MAX / sizeof(type)) ? NULL : \ ( (type *) PyMem_Malloc((n) * sizeof(type)) ) ) #define PyMem_NEW(type, n) \ - ( assert((n) <= PY_SIZE_MAX / sizeof(type)) , \ + ( ((n) > INT_MAX / sizeof(type)) ? NULL : \ ( (type *) PyMem_MALLOC((n) * sizeof(type)) ) ) +/* + * The value of (p) is always clobbered by this macro regardless of success. + * The caller MUST check if (p) is NULL afterwards and deal with the memory + * error if so. This means the original value of (p) MUST be saved for the + * caller's memory error handler to not lose track of it. + */ #define PyMem_Resize(p, type, n) \ - ( assert((n) <= PY_SIZE_MAX / sizeof(type)) , \ - ( (p) = (type *) PyMem_Realloc((p), (n) * sizeof(type)) ) ) + ( (p) = ((n) > INT_MAX / sizeof(type)) ? NULL : \ + (type *) PyMem_Realloc((p), (n) * sizeof(type)) ) #define PyMem_RESIZE(p, type, n) \ - ( assert((n) <= PY_SIZE_MAX / sizeof(type)) , \ - ( (p) = (type *) PyMem_REALLOC((p), (n) * sizeof(type)) ) ) + ( (p) = ((n) > INT_MAX / sizeof(type)) ? NULL : \ + (type *) PyMem_REALLOC((p), (n) * sizeof(type)) ) /* In order to avoid breaking old code mixing PyObject_{New, NEW} with PyMem_{Del, DEL} and PyMem_{Free, FREE}, the PyMem "release memory" diff --git a/Misc/NEWS b/Misc/NEWS index 9d8618acf532..517849208cfe 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -18,6 +18,13 @@ What's New in Python 2.4.5c1? Core and builtins ----------------- +- Issue #2620: Overflow checking when allocating or reallocating memory + was not always being done properly in some python types and extension + modules. PyMem_MALLOC, PyMem_REALLOC, PyMem_NEW and PyMem_RESIZE have + all been updated to perform better checks and places in the code that + would previously leak memory on the error path when such an allocation + failed have been fixed. + - Added checks for integer overflows, contributed by Google. Some are only available if asserts are left in the code, in cases where they can't be triggered from Python code. diff --git a/Modules/almodule.c b/Modules/almodule.c index fbeb13a63024..20f26d0ac530 100644 --- a/Modules/almodule.c +++ b/Modules/almodule.c @@ -1633,9 +1633,11 @@ al_QueryValues(PyObject *self, PyObject *args) if (nvals < 0) goto cleanup; if (nvals > setsize) { + ALvalue *old_return_set = return_set; setsize = nvals; PyMem_RESIZE(return_set, ALvalue, setsize); if (return_set == NULL) { + return_set = old_return_set; PyErr_NoMemory(); goto cleanup; } diff --git a/Modules/arraymodule.c b/Modules/arraymodule.c index aea4773e2546..f7ad5be5d1cb 100644 --- a/Modules/arraymodule.c +++ b/Modules/arraymodule.c @@ -814,6 +814,7 @@ static int array_do_extend(arrayobject *self, PyObject *bb) { int size; + char *old_item; if (!array_Check(bb)) return array_iter_extend(self, bb); @@ -829,10 +830,11 @@ array_do_extend(arrayobject *self, PyObject *bb) return -1; } size = self->ob_size + b->ob_size; + old_item = self->ob_item; PyMem_RESIZE(self->ob_item, char, size*self->ob_descr->itemsize); if (self->ob_item == NULL) { - PyObject_Del(self); - PyErr_NoMemory(); + self->ob_item = old_item; + PyErr_NoMemory(); return -1; } memcpy(self->ob_item + self->ob_size*self->ob_descr->itemsize, @@ -884,7 +886,7 @@ array_inplace_repeat(arrayobject *self, int n) if (size > INT_MAX / n) { return PyErr_NoMemory(); } - PyMem_Resize(items, char, n * size); + PyMem_RESIZE(items, char, n * size); if (items == NULL) return PyErr_NoMemory(); p = items; diff --git a/Modules/selectmodule.c b/Modules/selectmodule.c index 53c68c1a5598..25e23102b040 100644 --- a/Modules/selectmodule.c +++ b/Modules/selectmodule.c @@ -342,10 +342,12 @@ update_ufd_array(pollObject *self) { int i, pos; PyObject *key, *value; + struct pollfd *old_ufds = self->ufds; self->ufd_len = PyDict_Size(self->dict); - PyMem_Resize(self->ufds, struct pollfd, self->ufd_len); + PyMem_RESIZE(self->ufds, struct pollfd, self->ufd_len); if (self->ufds == NULL) { + self->ufds = old_ufds; PyErr_NoMemory(); return 0; } diff --git a/Objects/obmalloc.c b/Objects/obmalloc.c index a6fdf408112c..163c126bf8b2 100644 --- a/Objects/obmalloc.c +++ b/Objects/obmalloc.c @@ -584,6 +584,15 @@ PyObject_Malloc(size_t nbytes) poolp next; uint size; + /* + * Limit ourselves to INT_MAX bytes to prevent security holes. + * Most python internals blindly use a signed Py_ssize_t to track + * things without checking for overflows or negatives. + * As size_t is unsigned, checking for nbytes < 0 is not required. + */ + if (nbytes > INT_MAX) + return NULL; + /* * This implicitly redirects malloc(0). */ @@ -814,6 +823,15 @@ PyObject_Realloc(void *p, size_t nbytes) if (p == NULL) return PyObject_Malloc(nbytes); + /* + * Limit ourselves to INT_MAX bytes to prevent security holes. + * Most python internals blindly use a signed Py_ssize_t to track + * things without checking for overflows or negatives. + * As size_t is unsigned, checking for nbytes < 0 is not required. + */ + if (nbytes > INT_MAX) + return NULL; + pool = POOL_ADDR(p); if (Py_ADDRESS_IN_RANGE(p, pool)) { /* We're in charge of this block */