fromfile can segfault if data is corrupted
I just discovered a bug in fromfile where it can segfault if the file data is corrupted in such a way that the array size is insanely large. (It was a byte-swapping problem in my own code, but it would be preferable to get an exception rather than a crash). It's a simple fix to propagate the "array too large" exception before trying to dereference the NULL array pointer (ret) in PyArray_FromFile (see attached patch). But my question is: is this an appropriate fix for 1.4 (it seems pretty straightforward), or should I only make this to the trunk? Mike -- Michael Droettboom Science Software Branch Operations and Engineering Division Space Telescope Science Institute Operated by AURA for NASA Index: ctors.c =================================================================== --- ctors.c (revision 7992) +++ ctors.c (working copy) @@ -3042,6 +3055,10 @@ (next_element) fromfile_next_element, (skip_separator) fromfile_skip_separator, NULL); } + if (ret == NULL) { + Py_DECREF(dtype); + return NULL; + } if (((intp) nread) < num) { /* Realloc memory for smaller number of elements */ const size_t nsize = NPY_MAX(nread,1)*ret->descr->elsize;
On Tue, Dec 15, 2009 at 11:20 AM, Michael Droettboom <mdroe@stsci.edu>wrote:
I just discovered a bug in fromfile where it can segfault if the file data is corrupted in such a way that the array size is insanely large. (It was a byte-swapping problem in my own code, but it would be preferable to get an exception rather than a crash).
It's a simple fix to propagate the "array too large" exception before trying to dereference the NULL array pointer (ret) in PyArray_FromFile (see attached patch). But my question is: is this an appropriate fix for 1.4 (it seems pretty straightforward), or should I only make this to the trunk?
David can weigh in here, but I think you should backport it. It's a bugfix, small, and there is going to be another rc. On the other hand, Travis should stop backporting new functionality. <snip> Chuck
On Dec 15, 2009, at 12:28 PM, Charles R Harris wrote:
On Tue, Dec 15, 2009 at 11:20 AM, Michael Droettboom <mdroe@stsci.edu> wrote: I just discovered a bug in fromfile where it can segfault if the file data is corrupted in such a way that the array size is insanely large. (It was a byte-swapping problem in my own code, but it would be preferable to get an exception rather than a crash).
It's a simple fix to propagate the "array too large" exception before trying to dereference the NULL array pointer (ret) in PyArray_FromFile (see attached patch). But my question is: is this an appropriate fix for 1.4 (it seems pretty straightforward), or should I only make this to the trunk?
David can weigh in here, but I think you should backport it. It's a bugfix, small, and there is going to be another rc.
On the other hand, Travis should stop backporting new functionality.
And Chuck should stop making unrelated jabs.... I spoke with David C about making the change at SciPy India. It doesn't break any code and makes the datetime stuff in 1.4 more usable. In my mind datetime improvements are fair game for 1.4.0 until the release comes out. Or is there something else you are upset about and want to bring up on a public forum ? -Travis
On Fri, Dec 18, 2009 at 3:58 PM, Travis Oliphant <oliphant@enthought.com>wrote:
On Dec 15, 2009, at 12:28 PM, Charles R Harris wrote:
On Tue, Dec 15, 2009 at 11:20 AM, Michael Droettboom <mdroe@stsci.edu>wrote:
I just discovered a bug in fromfile where it can segfault if the file data is corrupted in such a way that the array size is insanely large. (It was a byte-swapping problem in my own code, but it would be preferable to get an exception rather than a crash).
It's a simple fix to propagate the "array too large" exception before trying to dereference the NULL array pointer (ret) in PyArray_FromFile (see attached patch). But my question is: is this an appropriate fix for 1.4 (it seems pretty straightforward), or should I only make this to the trunk?
David can weigh in here, but I think you should backport it. It's a bugfix, small, and there is going to be another rc.
On the other hand, Travis should stop backporting new functionality.
And Chuck should stop making unrelated jabs....
I spoke with David C about making the change at SciPy India. It doesn't break any code and makes the datetime stuff in 1.4 more usable. In my mind datetime improvements are fair game for 1.4.0 until the release comes out.
Or is there something else you are upset about and want to bring up on a public forum ?
Yes, backporting new code to a release candidate. If David signed off on it, then you should mention that in the commit comment. Chuck
participants (3)
-
Charles R Harris -
Michael Droettboom -
Travis Oliphant