diff options
| -rw-r--r-- | Doc/c-api/memory.rst | 4 | ||||
| -rw-r--r-- | Include/pymem.h | 33 | ||||
| -rw-r--r-- | Misc/NEWS | 7 | ||||
| -rw-r--r-- | Modules/almodule.c | 2 | ||||
| -rw-r--r-- | Modules/arraymodule.c | 5 | ||||
| -rw-r--r-- | Modules/selectmodule.c | 4 | ||||
| -rw-r--r-- | Objects/obmalloc.c | 18 |
7 files changed, 59 insertions, 14 deletions
diff --git a/Doc/c-api/memory.rst b/Doc/c-api/memory.rst index 1dcb115..81d7cd9 100644 --- a/Doc/c-api/memory.rst +++ b/Doc/c-api/memory.rst @@ -136,7 +136,9 @@ The following type-oriented macros are provided for convenience. Note that Same as :cfunc:`PyMem_Realloc`, but the memory block is resized to ``(n * sizeof(TYPE))`` bytes. Returns a pointer cast to :ctype:`TYPE\*`. On return, - *p* will be a pointer to the new memory area, or *NULL* in the event of failure. + *p* will be a pointer to the new memory area, or *NULL* in the event of + failure. This is a C preprocessor macro; p is always reassigned. Save + the original value of p to avoid losing memory when handling errors. .. cfunction:: void PyMem_Del(void *p) diff --git a/Include/pymem.h b/Include/pymem.h index f9acb55..542acee 100644 --- a/Include/pymem.h +++ b/Include/pymem.h @@ -69,8 +69,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) > PY_SSIZE_T_MAX) ? NULL \ + : malloc((n) ? (n) : 1)) +#define PyMem_REALLOC(p, n) (((n) < 0 || (n) > PY_SSIZE_T_MAX) ? NULL \ + : realloc((p), (n) ? (n) : 1)) #define PyMem_FREE free #endif /* PYMALLOC_DEBUG */ @@ -79,24 +83,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) > PY_SSIZE_T_MAX / sizeof(type)) ? NULL : \ ( (type *) PyMem_Malloc((n) * sizeof(type)) ) ) #define PyMem_NEW(type, n) \ - ( assert((n) <= PY_SIZE_MAX / sizeof(type)) , \ + ( ((n) > PY_SSIZE_T_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) > PY_SSIZE_T_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) > PY_SSIZE_T_MAX / sizeof(type)) ? NULL : \ + (type *) PyMem_REALLOC((p), (n) * sizeof(type)) ) /* PyMem{Del,DEL} are left over from ancient days, and shouldn't be used * anymore. They're just confusing aliases for PyMem_{Free,FREE} now. @@ -16,6 +16,13 @@ Core and Builtins the python debugger steps into a class statement: the free variables (local variables defined in an outer scope) would be deleted from the outer scope. +- 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. + Library ------- diff --git a/Modules/almodule.c b/Modules/almodule.c index 7f48fff..93f26bd 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 99d25d3..bcd82aa 100644 --- a/Modules/arraymodule.c +++ b/Modules/arraymodule.c @@ -815,6 +815,7 @@ static int array_do_extend(arrayobject *self, PyObject *bb) { Py_ssize_t size; + char *old_item; if (!array_Check(bb)) return array_iter_extend(self, bb); @@ -830,8 +831,10 @@ array_do_extend(arrayobject *self, PyObject *bb) return -1; } size = Py_SIZE(self) + Py_SIZE(b); + old_item = self->ob_item; PyMem_RESIZE(self->ob_item, char, size*self->ob_descr->itemsize); if (self->ob_item == NULL) { + self->ob_item = old_item; PyErr_NoMemory(); return -1; } @@ -884,7 +887,7 @@ array_inplace_repeat(arrayobject *self, Py_ssize_t n) if (size > PY_SSIZE_T_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 83a6538..86ee3cc 100644 --- a/Modules/selectmodule.c +++ b/Modules/selectmodule.c @@ -349,10 +349,12 @@ update_ufd_array(pollObject *self) { Py_ssize_t 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 efbd566..da8f9c2 100644 --- a/Objects/obmalloc.c +++ b/Objects/obmalloc.c @@ -727,6 +727,15 @@ PyObject_Malloc(size_t nbytes) uint size; /* + * Limit ourselves to PY_SSIZE_T_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 > PY_SSIZE_T_MAX) + return NULL; + + /* * This implicitly redirects malloc(0). */ if ((nbytes - 1) < SMALL_REQUEST_THRESHOLD) { @@ -1130,6 +1139,15 @@ PyObject_Realloc(void *p, size_t nbytes) if (p == NULL) return PyObject_Malloc(nbytes); + /* + * Limit ourselves to PY_SSIZE_T_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 > PY_SSIZE_T_MAX) + return NULL; + pool = POOL_ADDR(p); if (Py_ADDRESS_IN_RANGE(p, pool)) { /* We're in charge of this block */ |