Commit cd04db03 authored by Benjamin Peterson's avatar Benjamin Peterson

mmap: do all internal arithmetic with Py_ssize_t while being very careful about overflow

parent 92e7c7f9
...@@ -713,6 +713,17 @@ class MmapTests(unittest.TestCase): ...@@ -713,6 +713,17 @@ class MmapTests(unittest.TestCase):
gc_collect() gc_collect()
self.assertIs(wr(), None) self.assertIs(wr(), None)
def test_resize_past_pos(self):
m = mmap.mmap(-1, 8192)
self.addCleanup(m.close)
m.read(5000)
m.resize(4096)
self.assertEqual(m.read(14), b'')
self.assertRaises(ValueError, m.read_byte,)
self.assertRaises(ValueError, m.write_byte, 42)
self.assertRaises(ValueError, m.write, b'abc')
class LargeMmapTests(unittest.TestCase): class LargeMmapTests(unittest.TestCase):
def setUp(self): def setUp(self):
......
...@@ -94,6 +94,9 @@ Library ...@@ -94,6 +94,9 @@ Library
- Issue #28322: Fixed possible crashes when unpickle itertools objects from - Issue #28322: Fixed possible crashes when unpickle itertools objects from
incorrect pickle data. Based on patch by John Leitch. incorrect pickle data. Based on patch by John Leitch.
- Fix possible integer overflows and crashes in the mmap module with unusual
usage patterns.
- Issue #1703178: Fix the ability to pass the --link-objects option to the - Issue #1703178: Fix the ability to pass the --link-objects option to the
distutils build_ext command. distutils build_ext command.
......
...@@ -90,8 +90,8 @@ typedef enum ...@@ -90,8 +90,8 @@ typedef enum
typedef struct { typedef struct {
PyObject_HEAD PyObject_HEAD
char * data; char * data;
size_t size; Py_ssize_t size;
size_t pos; /* relative to offset */ Py_ssize_t pos; /* relative to offset */
#ifdef MS_WINDOWS #ifdef MS_WINDOWS
PY_LONG_LONG offset; PY_LONG_LONG offset;
#else #else
...@@ -210,33 +210,32 @@ mmap_read_byte_method(mmap_object *self, ...@@ -210,33 +210,32 @@ mmap_read_byte_method(mmap_object *self,
PyObject *unused) PyObject *unused)
{ {
CHECK_VALID(NULL); CHECK_VALID(NULL);
if (self->pos < self->size) { if (self->pos >= self->size) {
char value = self->data[self->pos];
self->pos += 1;
return Py_BuildValue("B", (unsigned char)value);
} else {
PyErr_SetString(PyExc_ValueError, "read byte out of range"); PyErr_SetString(PyExc_ValueError, "read byte out of range");
return NULL; return NULL;
} }
return PyLong_FromLong((unsigned char)self->data[self->pos++]);
} }
static PyObject * static PyObject *
mmap_read_line_method(mmap_object *self, mmap_read_line_method(mmap_object *self,
PyObject *unused) PyObject *unused)
{ {
char *start = self->data+self->pos; Py_ssize_t remaining;
char *eof = self->data+self->size; char *start, *eol;
char *eol;
PyObject *result; PyObject *result;
CHECK_VALID(NULL); CHECK_VALID(NULL);
eol = memchr(start, '\n', self->size - self->pos); remaining = (self->pos < self->size) ? self->size - self->pos : 0;
if (!remaining)
return PyBytes_FromString("");
start = self->data + self->pos;
eol = memchr(start, '\n', remaining);
if (!eol) if (!eol)
eol = eof; eol = self->data + self->size;
else else
++eol; /* we're interested in the position after the ++eol; /* advance past newline */
newline. */
result = PyBytes_FromStringAndSize(start, (eol - start)); result = PyBytes_FromStringAndSize(start, (eol - start));
self->pos += (eol - start); self->pos += (eol - start);
return result; return result;
...@@ -268,7 +267,7 @@ static PyObject * ...@@ -268,7 +267,7 @@ static PyObject *
mmap_read_method(mmap_object *self, mmap_read_method(mmap_object *self,
PyObject *args) PyObject *args)
{ {
Py_ssize_t num_bytes = -1, n; Py_ssize_t num_bytes, remaining;
PyObject *result; PyObject *result;
CHECK_VALID(NULL); CHECK_VALID(NULL);
...@@ -276,20 +275,10 @@ mmap_read_method(mmap_object *self, ...@@ -276,20 +275,10 @@ mmap_read_method(mmap_object *self,
return(NULL); return(NULL);
/* silently 'adjust' out-of-range requests */ /* silently 'adjust' out-of-range requests */
assert(self->size >= self->pos); remaining = (self->pos < self->size) ? self->size - self->pos : 0;
n = self->size - self->pos; if (num_bytes < 0 || num_bytes > remaining)
/* The difference can overflow, only if self->size is greater than num_bytes = remaining;
* PY_SSIZE_T_MAX. But then the operation cannot possibly succeed, result = PyBytes_FromStringAndSize(&self->data[self->pos], num_bytes);
* because the mapped area and the returned string each need more
* than half of the addressable memory. So we clip the size, and let
* the code below raise MemoryError.
*/
if (n < 0)
n = PY_SSIZE_T_MAX;
if (num_bytes < 0 || num_bytes > n) {
num_bytes = n;
}
result = PyBytes_FromStringAndSize(self->data+self->pos, num_bytes);
self->pos += num_bytes; self->pos += num_bytes;
return result; return result;
} }
...@@ -317,14 +306,14 @@ mmap_gfind(mmap_object *self, ...@@ -317,14 +306,14 @@ mmap_gfind(mmap_object *self,
start += self->size; start += self->size;
if (start < 0) if (start < 0)
start = 0; start = 0;
else if ((size_t)start > self->size) else if (start > self->size)
start = self->size; start = self->size;
if (end < 0) if (end < 0)
end += self->size; end += self->size;
if (end < 0) if (end < 0)
end = 0; end = 0;
else if ((size_t)end > self->size) else if (end > self->size)
end = self->size; end = self->size;
start_p = self->data + start; start_p = self->data + start;
...@@ -394,18 +383,17 @@ mmap_write_method(mmap_object *self, ...@@ -394,18 +383,17 @@ mmap_write_method(mmap_object *self,
if (!PyArg_ParseTuple(args, "y*:write", &data)) if (!PyArg_ParseTuple(args, "y*:write", &data))
return(NULL); return(NULL);
if (!is_writable(self)) { if (!is_writable(self))
PyBuffer_Release(&data);
return NULL; return NULL;
}
if ((self->pos + data.len) > self->size) { if (self->pos > self->size || self->size - self->pos < data.len) {
PyErr_SetString(PyExc_ValueError, "data out of range");
PyBuffer_Release(&data); PyBuffer_Release(&data);
PyErr_SetString(PyExc_ValueError, "data out of range");
return NULL; return NULL;
} }
memcpy(self->data + self->pos, data.buf, data.len);
self->pos = self->pos + data.len; memcpy(&self->data[self->pos], data.buf, data.len);
self->pos += data.len;
PyBuffer_Release(&data); PyBuffer_Release(&data);
Py_INCREF(Py_None); Py_INCREF(Py_None);
return Py_None; return Py_None;
...@@ -425,8 +413,7 @@ mmap_write_byte_method(mmap_object *self, ...@@ -425,8 +413,7 @@ mmap_write_byte_method(mmap_object *self,
return NULL; return NULL;
if (self->pos < self->size) { if (self->pos < self->size) {
*(self->data+self->pos) = value; self->data[self->pos++] = value;
self->pos += 1;
Py_INCREF(Py_None); Py_INCREF(Py_None);
return Py_None; return Py_None;
} }
...@@ -495,8 +482,14 @@ mmap_resize_method(mmap_object *self, ...@@ -495,8 +482,14 @@ mmap_resize_method(mmap_object *self,
if (!PyArg_ParseTuple(args, "n:resize", &new_size) || if (!PyArg_ParseTuple(args, "n:resize", &new_size) ||
!is_resizeable(self)) { !is_resizeable(self)) {
return NULL; return NULL;
}
if (new_size < 0 || PY_SSIZE_T_MAX - new_size < self->offset) {
PyErr_SetString(PyExc_ValueError, "new size out of range");
return NULL;
}
{
#ifdef MS_WINDOWS #ifdef MS_WINDOWS
} else {
DWORD dwErrCode = 0; DWORD dwErrCode = 0;
DWORD off_hi, off_lo, newSizeLow, newSizeHigh; DWORD off_hi, off_lo, newSizeLow, newSizeHigh;
/* First, unmap the file view */ /* First, unmap the file view */
...@@ -546,15 +539,13 @@ mmap_resize_method(mmap_object *self, ...@@ -546,15 +539,13 @@ mmap_resize_method(mmap_object *self,
#ifdef UNIX #ifdef UNIX
#ifndef HAVE_MREMAP #ifndef HAVE_MREMAP
} else {
PyErr_SetString(PyExc_SystemError, PyErr_SetString(PyExc_SystemError,
"mmap: resizing not available--no mremap()"); "mmap: resizing not available--no mremap()");
return NULL; return NULL;
#else #else
} else {
void *newmap; void *newmap;
if (ftruncate(self->fd, self->offset + new_size) == -1) { if (self->fd != -1 && ftruncate(self->fd, self->offset + new_size) == -1) {
PyErr_SetFromErrno(PyExc_OSError); PyErr_SetFromErrno(PyExc_OSError);
return NULL; return NULL;
} }
...@@ -562,11 +553,11 @@ mmap_resize_method(mmap_object *self, ...@@ -562,11 +553,11 @@ mmap_resize_method(mmap_object *self,
#ifdef MREMAP_MAYMOVE #ifdef MREMAP_MAYMOVE
newmap = mremap(self->data, self->size, new_size, MREMAP_MAYMOVE); newmap = mremap(self->data, self->size, new_size, MREMAP_MAYMOVE);
#else #else
#if defined(__NetBSD__) #if defined(__NetBSD__)
newmap = mremap(self->data, self->size, self->data, new_size, 0); newmap = mremap(self->data, self->size, self->data, new_size, 0);
#else #else
newmap = mremap(self->data, self->size, new_size, 0); newmap = mremap(self->data, self->size, new_size, 0);
#endif /* __NetBSD__ */ #endif /* __NetBSD__ */
#endif #endif
if (newmap == (void *)-1) if (newmap == (void *)-1)
{ {
...@@ -597,7 +588,7 @@ mmap_flush_method(mmap_object *self, PyObject *args) ...@@ -597,7 +588,7 @@ mmap_flush_method(mmap_object *self, PyObject *args)
CHECK_VALID(NULL); CHECK_VALID(NULL);
if (!PyArg_ParseTuple(args, "|nn:flush", &offset, &size)) if (!PyArg_ParseTuple(args, "|nn:flush", &offset, &size))
return NULL; return NULL;
if ((size_t)(offset + size) > self->size) { if (size < 0 || offset < 0 || self->size - offset < size) {
PyErr_SetString(PyExc_ValueError, "flush values out of range"); PyErr_SetString(PyExc_ValueError, "flush values out of range");
return NULL; return NULL;
} }
...@@ -630,20 +621,18 @@ mmap_seek_method(mmap_object *self, PyObject *args) ...@@ -630,20 +621,18 @@ mmap_seek_method(mmap_object *self, PyObject *args)
if (!PyArg_ParseTuple(args, "n|i:seek", &dist, &how)) if (!PyArg_ParseTuple(args, "n|i:seek", &dist, &how))
return NULL; return NULL;
else { else {
size_t where; Py_ssize_t where;
switch (how) { switch (how) {
case 0: /* relative to start */ case 0: /* relative to start */
if (dist < 0)
goto onoutofrange;
where = dist; where = dist;
break; break;
case 1: /* relative to current position */ case 1: /* relative to current position */
if ((Py_ssize_t)self->pos + dist < 0) if (PY_SSIZE_T_MAX - self->pos < dist)
goto onoutofrange; goto onoutofrange;
where = self->pos + dist; where = self->pos + dist;
break; break;
case 2: /* relative to end */ case 2: /* relative to end */
if ((Py_ssize_t)self->size + dist < 0) if (PY_SSIZE_T_MAX - self->size < dist)
goto onoutofrange; goto onoutofrange;
where = self->size + dist; where = self->size + dist;
break; break;
...@@ -651,7 +640,7 @@ mmap_seek_method(mmap_object *self, PyObject *args) ...@@ -651,7 +640,7 @@ mmap_seek_method(mmap_object *self, PyObject *args)
PyErr_SetString(PyExc_ValueError, "unknown seek type"); PyErr_SetString(PyExc_ValueError, "unknown seek type");
return NULL; return NULL;
} }
if (where > self->size) if (where > self->size || where < 0)
goto onoutofrange; goto onoutofrange;
self->pos = where; self->pos = where;
Py_INCREF(Py_None); Py_INCREF(Py_None);
...@@ -666,24 +655,28 @@ mmap_seek_method(mmap_object *self, PyObject *args) ...@@ -666,24 +655,28 @@ mmap_seek_method(mmap_object *self, PyObject *args)
static PyObject * static PyObject *
mmap_move_method(mmap_object *self, PyObject *args) mmap_move_method(mmap_object *self, PyObject *args)
{ {
unsigned long dest, src, cnt; Py_ssize_t dest, src, cnt;
CHECK_VALID(NULL); CHECK_VALID(NULL);
if (!PyArg_ParseTuple(args, "kkk:move", &dest, &src, &cnt) || if (!PyArg_ParseTuple(args, "nnn:move", &dest, &src, &cnt) ||
!is_writable(self)) { !is_writable(self)) {
return NULL; return NULL;
} else { } else {
/* bounds check the values */ /* bounds check the values */
if ((cnt + dest) < cnt || (cnt + src) < cnt || if (dest < 0 || src < 0 || cnt < 0)
src > self->size || (src + cnt) > self->size || goto bounds;
dest > self->size || (dest + cnt) > self->size) { if (self->size - dest < cnt || self->size - src < cnt)
goto bounds;
memmove(&self->data[dest], &self->data[src], cnt);
Py_INCREF(Py_None);
return Py_None;
bounds:
PyErr_SetString(PyExc_ValueError, PyErr_SetString(PyExc_ValueError,
"source, destination, or count out of range"); "source, destination, or count out of range");
return NULL; return NULL;
} }
memmove(self->data+dest, self->data+src, cnt);
Py_INCREF(Py_None);
return Py_None;
}
} }
static PyObject * static PyObject *
...@@ -785,7 +778,7 @@ static PyObject * ...@@ -785,7 +778,7 @@ static PyObject *
mmap_item(mmap_object *self, Py_ssize_t i) mmap_item(mmap_object *self, Py_ssize_t i)
{ {
CHECK_VALID(NULL); CHECK_VALID(NULL);
if (i < 0 || (size_t)i >= self->size) { if (i < 0 || i >= self->size) {
PyErr_SetString(PyExc_IndexError, "mmap index out of range"); PyErr_SetString(PyExc_IndexError, "mmap index out of range");
return NULL; return NULL;
} }
...@@ -802,7 +795,7 @@ mmap_subscript(mmap_object *self, PyObject *item) ...@@ -802,7 +795,7 @@ mmap_subscript(mmap_object *self, PyObject *item)
return NULL; return NULL;
if (i < 0) if (i < 0)
i += self->size; i += self->size;
if (i < 0 || (size_t)i >= self->size) { if (i < 0 || i >= self->size) {
PyErr_SetString(PyExc_IndexError, PyErr_SetString(PyExc_IndexError,
"mmap index out of range"); "mmap index out of range");
return NULL; return NULL;
...@@ -870,7 +863,7 @@ mmap_ass_item(mmap_object *self, Py_ssize_t i, PyObject *v) ...@@ -870,7 +863,7 @@ mmap_ass_item(mmap_object *self, Py_ssize_t i, PyObject *v)
const char *buf; const char *buf;
CHECK_VALID(-1); CHECK_VALID(-1);
if (i < 0 || (size_t)i >= self->size) { if (i < 0 || i >= self->size) {
PyErr_SetString(PyExc_IndexError, "mmap index out of range"); PyErr_SetString(PyExc_IndexError, "mmap index out of range");
return -1; return -1;
} }
...@@ -907,7 +900,7 @@ mmap_ass_subscript(mmap_object *self, PyObject *item, PyObject *value) ...@@ -907,7 +900,7 @@ mmap_ass_subscript(mmap_object *self, PyObject *item, PyObject *value)
return -1; return -1;
if (i < 0) if (i < 0)
i += self->size; i += self->size;
if (i < 0 || (size_t)i >= self->size) { if (i < 0 || i >= self->size) {
PyErr_SetString(PyExc_IndexError, PyErr_SetString(PyExc_IndexError,
"mmap index out of range"); "mmap index out of range");
return -1; return -1;
...@@ -1074,32 +1067,6 @@ static PyTypeObject mmap_object_type = { ...@@ -1074,32 +1067,6 @@ static PyTypeObject mmap_object_type = {
}; };
/* extract the map size from the given PyObject
Returns -1 on error, with an appropriate Python exception raised. On
success, the map size is returned. */
static Py_ssize_t
_GetMapSize(PyObject *o, const char* param)
{
if (o == NULL)
return 0;
if (PyIndex_Check(o)) {
Py_ssize_t i = PyNumber_AsSsize_t(o, PyExc_OverflowError);
if (i==-1 && PyErr_Occurred())
return -1;
if (i < 0) {
PyErr_Format(PyExc_OverflowError,
"memory mapped %s must be positive",
param);
return -1;
}
return i;
}
PyErr_SetString(PyExc_TypeError, "map size must be an integral value");
return -1;
}
#ifdef UNIX #ifdef UNIX
#ifdef HAVE_LARGEFILE_SUPPORT #ifdef HAVE_LARGEFILE_SUPPORT
#define _Py_PARSE_OFF_T "L" #define _Py_PARSE_OFF_T "L"
...@@ -1112,7 +1079,6 @@ new_mmap_object(PyTypeObject *type, PyObject *args, PyObject *kwdict) ...@@ -1112,7 +1079,6 @@ new_mmap_object(PyTypeObject *type, PyObject *args, PyObject *kwdict)
{ {
struct _Py_stat_struct status; struct _Py_stat_struct status;
mmap_object *m_obj; mmap_object *m_obj;
PyObject *map_size_obj = NULL;
Py_ssize_t map_size; Py_ssize_t map_size;
off_t offset = 0; off_t offset = 0;
int fd, flags = MAP_SHARED, prot = PROT_WRITE | PROT_READ; int fd, flags = MAP_SHARED, prot = PROT_WRITE | PROT_READ;
...@@ -1122,13 +1088,15 @@ new_mmap_object(PyTypeObject *type, PyObject *args, PyObject *kwdict) ...@@ -1122,13 +1088,15 @@ new_mmap_object(PyTypeObject *type, PyObject *args, PyObject *kwdict)
"flags", "prot", "flags", "prot",
"access", "offset", NULL}; "access", "offset", NULL};
if (!PyArg_ParseTupleAndKeywords(args, kwdict, "iO|iii" _Py_PARSE_OFF_T, keywords, if (!PyArg_ParseTupleAndKeywords(args, kwdict, "in|iii" _Py_PARSE_OFF_T, keywords,
&fd, &map_size_obj, &flags, &prot, &fd, &map_size, &flags, &prot,
&access, &offset)) &access, &offset))
return NULL; return NULL;
map_size = _GetMapSize(map_size_obj, "size"); if (map_size < 0) {
if (map_size < 0) PyErr_SetString(PyExc_OverflowError,
"memory mapped length must be postiive");
return NULL; return NULL;
}
if (offset < 0) { if (offset < 0) {
PyErr_SetString(PyExc_OverflowError, PyErr_SetString(PyExc_OverflowError,
"memory mapped offset must be positive"); "memory mapped offset must be positive");
...@@ -1194,7 +1162,7 @@ new_mmap_object(PyTypeObject *type, PyObject *args, PyObject *kwdict) ...@@ -1194,7 +1162,7 @@ new_mmap_object(PyTypeObject *type, PyObject *args, PyObject *kwdict)
return NULL; return NULL;
} }
map_size = (Py_ssize_t) (status.st_size - offset); map_size = (Py_ssize_t) (status.st_size - offset);
} else if (offset + map_size > status.st_size) { } else if (offset > status.st_size || status.st_size - offset < map_size) {
PyErr_SetString(PyExc_ValueError, PyErr_SetString(PyExc_ValueError,
"mmap length is greater than file size"); "mmap length is greater than file size");
return NULL; return NULL;
...@@ -1203,8 +1171,8 @@ new_mmap_object(PyTypeObject *type, PyObject *args, PyObject *kwdict) ...@@ -1203,8 +1171,8 @@ new_mmap_object(PyTypeObject *type, PyObject *args, PyObject *kwdict)
m_obj = (mmap_object *)type->tp_alloc(type, 0); m_obj = (mmap_object *)type->tp_alloc(type, 0);
if (m_obj == NULL) {return NULL;} if (m_obj == NULL) {return NULL;}
m_obj->data = NULL; m_obj->data = NULL;
m_obj->size = (size_t) map_size; m_obj->size = map_size;
m_obj->pos = (size_t) 0; m_obj->pos = 0;
m_obj->weakreflist = NULL; m_obj->weakreflist = NULL;
m_obj->exports = 0; m_obj->exports = 0;
m_obj->offset = offset; m_obj->offset = offset;
...@@ -1264,7 +1232,6 @@ static PyObject * ...@@ -1264,7 +1232,6 @@ static PyObject *
new_mmap_object(PyTypeObject *type, PyObject *args, PyObject *kwdict) new_mmap_object(PyTypeObject *type, PyObject *args, PyObject *kwdict)
{ {
mmap_object *m_obj; mmap_object *m_obj;
PyObject *map_size_obj = NULL;
Py_ssize_t map_size; Py_ssize_t map_size;
PY_LONG_LONG offset = 0, size; PY_LONG_LONG offset = 0, size;
DWORD off_hi; /* upper 32 bits of offset */ DWORD off_hi; /* upper 32 bits of offset */
...@@ -1281,8 +1248,8 @@ new_mmap_object(PyTypeObject *type, PyObject *args, PyObject *kwdict) ...@@ -1281,8 +1248,8 @@ new_mmap_object(PyTypeObject *type, PyObject *args, PyObject *kwdict)
"tagname", "tagname",
"access", "offset", NULL }; "access", "offset", NULL };
if (!PyArg_ParseTupleAndKeywords(args, kwdict, "iO|ziL", keywords, if (!PyArg_ParseTupleAndKeywords(args, kwdict, "in|ziL", keywords,
&fileno, &map_size_obj, &fileno, &map_size,
&tagname, &access, &offset)) { &tagname, &access, &offset)) {
return NULL; return NULL;
} }
...@@ -1305,9 +1272,11 @@ new_mmap_object(PyTypeObject *type, PyObject *args, PyObject *kwdict) ...@@ -1305,9 +1272,11 @@ new_mmap_object(PyTypeObject *type, PyObject *args, PyObject *kwdict)
"mmap invalid access parameter."); "mmap invalid access parameter.");
} }
map_size = _GetMapSize(map_size_obj, "size"); if (map_size < 0) {
if (map_size < 0) PyErr_SetString(PyExc_OverflowError,
"memory mapped length must be postiive");
return NULL; return NULL;
}
if (offset < 0) { if (offset < 0) {
PyErr_SetString(PyExc_OverflowError, PyErr_SetString(PyExc_OverflowError,
"memory mapped offset must be positive"); "memory mapped offset must be positive");
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment