bounds check arguments to mmap.move(). All of them. Really.

fixes crasher on OS X 10.5

diff --git a/Lib/test/test_mmap.py b/Lib/test/test_mmap.py
index 1a23f2e4672a8e34553491f5f3b1a3dcfa787cc4..d6c62c192f74ebd5611d8a756c8c4c75d68d6b6c 100644 (file)
--- a/Lib/test/test_mmap.py
+++ b/Lib/test/test_mmap.py
@@ -359,15 +359,22 @@ class MmapTests(unittest.TestCase):
                 m.move(source, dest, size)
             except ValueError:
                 pass
-        self.assertRaises(ValueError, m.move, -1, -1, -1)
-        self.assertRaises(ValueError, m.move, -1, -1, 0)
-        self.assertRaises(ValueError, m.move, -1, 0, -1)
-        self.assertRaises(ValueError, m.move, 0, -1, -1)
-        self.assertRaises(ValueError, m.move, -1, 0, 0)
-        self.assertRaises(ValueError, m.move, 0, -1, 0)
-        self.assertRaises(ValueError, m.move, 0, 0, -1)
+
+        offsets = [(-1, -1, -1), (-1, -1, 0), (-1, 0, -1), (0, -1, -1),
+                   (-1, 0, 0), (0, -1, 0), (0, 0, -1)]
+        for source, dest, size in offsets:
+            self.assertRaises(ValueError, m.move, source, dest, size)
+
         m.close()
 
+        m = mmap.mmap(-1, 1) # single byte
+        self.assertRaises(ValueError, m.move, 0, 0, 2)
+        self.assertRaises(ValueError, m.move, 1, 0, 1)
+        self.assertRaises(ValueError, m.move, 0, 1, 1)
+        m.move(0, 0, 1)
+        m.move(0, 0, 0)
+
+
     def test_anonymous(self):
         # anonymous mmap.mmap(-1, PAGE)
         m = mmap.mmap(-1, PAGESIZE)

diff --git a/Modules/mmapmodule.c b/Modules/mmapmodule.c
index cbacc2fc25222bcd5a20d739d43731d907286ca6..f660d6bb0ba2f138b1a647543648e2a1dc5b9659 100644 (file)
--- a/Modules/mmapmodule.c
+++ b/Modules/mmapmodule.c
@@ -609,23 +609,23 @@ mmap_seek_method(mmap_object *self, PyObject *args)
 static PyObject *
 mmap_move_method(mmap_object *self, PyObject *args)
 {
-       unsigned long dest, src, count;
+       unsigned long dest, src, cnt;
        CHECK_VALID(NULL);
-       if (!PyArg_ParseTuple(args, "kkk:move", &dest, &src, &count) ||
+       if (!PyArg_ParseTuple(args, "kkk:move", &dest, &src, &cnt) ||
            !is_writeable(self)) {
                return NULL;
        } else {
                /* bounds check the values */
-               unsigned long pos = src > dest ? src : dest;
-               if (self->size < pos || count > self->size - pos) {
+               if (cnt < 0 || (cnt + dest) < cnt || (cnt + src) < cnt ||
+                  src < 0 || src > self->size || (src + cnt) > self->size ||
+                  dest < 0 || dest > self->size || (dest + cnt) > self->size) {
                        PyErr_SetString(PyExc_ValueError,
-                                       "source or destination out of range");
+                               "source, destination, or count out of range");
                        return NULL;
-               } else {
-                       memmove(self->data+dest, self->data+src, count);
-                       Py_INCREF(Py_None);
-                       return Py_None;
                }
+               memmove(self->data+dest, self->data+src, cnt);
+               Py_INCREF(Py_None);
+               return Py_None;
        }
 }