-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
MNT: protect from out-of-bounds data access at the c level #14478
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
(0, 2, 1, 6), | ||
): | ||
with pytest.raises(ValueError): | ||
print(bad_boxes) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove the print?
src/_tkagg.cpp
Outdated
@@ -67,6 +67,12 @@ static PyObject *mpl_tk_blit(PyObject *self, PyObject *args) | |||
PyErr_SetString(PyExc_ValueError, "Failed to extract Tk_PhotoHandle"); | |||
goto exit; | |||
} | |||
if (0 > y1 || y1 > y2 || y2 > height || | |||
0 > x1 || x1 > x2 || x2 > width ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
extra space after width?
- due to missing tk libraries - due to qt already being imported
I still can write code I swear... |
Can probably re-use the |
quite possibly, indeed, looks like a good idea. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor comment, but would also be ok without.
@@ -67,6 +67,11 @@ static PyObject *mpl_tk_blit(PyObject *self, PyObject *args) | |||
PyErr_SetString(PyExc_ValueError, "Failed to extract Tk_PhotoHandle"); | |||
goto exit; | |||
} | |||
if (0 > y1 || y1 > y2 || y2 > height || 0 > x1 || x1 > x2 || x2 > width) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (0 > y1 || y1 > y2 || y2 > height || 0 > x1 || x1 > x2 || x2 > width) { | |
if (y1 < 0 || y1 > y2 || y2 > height || x1 < 0 || x1 > x2 || x2 > width) { |
I find it easier this way "y1 is smaller than 0 or larger than y2". 0 > y1
sort of ties a knot in my brain. 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is fair, but having all of the >
go the same way is also helpful..
Moot now that @efiring merged it..
As suggested by @cgohlke
PR Summary
PR Checklist