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 0e18f03d0126578cd4b21bb960f9a6ceb6aeb43a..a3eb0955a724ad9d29fa5eb9b588792164513af9 100644 (file)
--- 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 9d8618acf532302c4f4da2fc3558dff2e963aba2..517849208cfedb157dbb5523ba2de2de130c09ea 100644 (file)
--- 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 fbeb13a630245bdc40f0f5c3486468f857159164..20f26d0ac530102380122c8347b8fe7495a797ef 100644 (file)
--- 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 aea4773e254690591622eb2c4d638c4020fb0783..f7ad5be5d1cbd7c250607106038839f04a70775c 100644 (file)
--- 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 53c68c1a55983959c6019098ce132537153a6002..25e23102b040f32a7ee117aca298075f99b31266 100644 (file)
--- 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 a6fdf408112cc57cefca685082d9977b66995f73..163c126bf8b2c73a356247aad014cbffd3dd86ba 100644 (file)
--- 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 */