bpo-33191: Fix refleak in posix_spawn#6315
bpo-33191: Fix refleak in posix_spawn#6315zware wants to merge 3 commits into
Conversation
|
|
||
| if(!PySequence_Check(file_actions_obj) | !PySequence_Size(file_actions_obj)) { | ||
| PyErr_SetString(PyExc_TypeError,"Each file_action element must be a non empty sequence"); | ||
| if (file_actions_obj == NULL) { |
| if (file_actions_obj == NULL) { | ||
| goto exit; | ||
| } | ||
| file_actions_obj = PySequence_Fast(file_actions_obj, "Each file_action element must be a non-empty sequence"); |
There was a problem hiding this comment.
Would be nice to wrap this long line.
|
|
||
| long open_fd = PyLong_AsLong(PySequence_GetItem(file_actions_obj, 1)); | ||
| long open_fd = PyLong_AsLong(PySequence_Fast_GET_ITEM(file_actions_obj, 1)); | ||
| if(PyErr_Occurred()) { |
There was a problem hiding this comment.
While you are here, could you please make the source conforming PEP 7: Add spaces after "if" and "switch", wrap long lines?
| } | ||
|
|
||
| long open_fd = PyLong_AsLong(PySequence_GetItem(file_actions_obj, 1)); | ||
| long open_fd = PyLong_AsLong(PySequence_Fast_GET_ITEM(file_actions_obj, 1)); |
There was a problem hiding this comment.
PyLong_AsLong() can call arbitrary code. If file_actions is a list, the size of file_actions_obj can be changed after this, and the following PySequence_Fast_GET_ITEM() could read past the end of the list.
I would require file_actions to be a tuple and use PyArg_ParseTuple() for parsing it.
|
I have added more comments in #5109 because it is hard to add comment to the code not changed in a PR. |
|
@serhiy-storchaka Would you mind to take this over? I would like to see this fixed before 3.7.0 final, but don't know when I'll be able to get back to it. |
|
I have opened #6331 trying to address al the issues at once. |
|
#6332 should obsolete this |
https://bugs.python.org/issue33191