DiffHunk_lines__get__() and Patch_hunks__get__() use the result of PyList_New() without checking it for NULL. On allocation failure both write through a NULL pointer and segfault instead of raising MemoryError. The same two loops also leak the partially filled list when a per-item call fails.
Files: src/diff.c, src/patch.c
Functions: DiffHunk_lines__get__ (src/diff.c, line 766), Patch_hunks__get__ (src/patch.c, line 218)
py_lines = PyList_New(self->n_lines);
for (i = 0; i < self->n_lines; ++i) {
err = git_patch_get_line_in_hunk(&line, self->patch->patch, self->idx, i);
if (err < 0)
return Error_set(err); /* py_lines leaked */
py_line = wrap_diff_line(line, self);
if (py_line == NULL)
return NULL; /* py_lines leaked */
PyList_SetItem(py_lines, i, py_line);
}
return py_lines;
hunk_amounts = git_patch_num_hunks(self->patch);
py_hunks = PyList_New(hunk_amounts);
for (i = 0; i < hunk_amounts; i++) {
py_hunk = wrap_diff_hunk(self, i);
if (py_hunk == NULL)
return NULL; /* py_hunks leaked */
PyList_SET_ITEM((PyObject*) py_hunks, i, py_hunk);
}
return py_hunks;
PyList_New() returns NULL on allocation failure, and neither function checks. Both then crash rather than propagating MemoryError:
PyList_SET_ITEM((PyObject*) py_hunks, i, py_hunk) is a macro that expands to _PyList_CAST(op)->ob_item[index] = value, so it writes through NULL plus an offset.
PyList_SetItem(py_lines, i, py_line) starts with PyList_Check(op), which evaluates Py_TYPE(op) and dereferences NULL.
The trigger is an allocation failure, so this is not reachable from ordinary code, but the failure mode is a segfault rather than a recoverable exception.
Both loops return NULL mid-iteration without releasing the list they already own. In DiffHunk_lines__get__ there are two such paths (git_patch_get_line_in_hunk failing, and wrap_diff_line returning NULL); in Patch_hunks__get__ there is one.
Suggested fix
Check the allocation and release the list on every early return:
py_lines = PyList_New(self->n_lines);
if (py_lines == NULL)
return NULL;
for (i = 0; i < self->n_lines; ++i) {
err = git_patch_get_line_in_hunk(&line, self->patch->patch, self->idx, i);
if (err < 0) {
Py_DECREF(py_lines);
return Error_set(err);
}
py_line = wrap_diff_line(line, self);
if (py_line == NULL) {
Py_DECREF(py_lines);
return NULL;
}
PyList_SetItem(py_lines, i, py_line);
}
return py_lines;
Apply the equivalent shape to Patch_hunks__get__.
DiffHunk_lines__get__()andPatch_hunks__get__()use the result ofPyList_New()without checking it forNULL. On allocation failure both write through aNULLpointer and segfault instead of raisingMemoryError. The same two loops also leak the partially filled list when a per-item call fails.Files:
src/diff.c,src/patch.cFunctions:
DiffHunk_lines__get__(src/diff.c, line 766),Patch_hunks__get__(src/patch.c, line 218)PyList_New()returnsNULLon allocation failure, and neither function checks. Both then crash rather than propagatingMemoryError:PyList_SET_ITEM((PyObject*) py_hunks, i, py_hunk)is a macro that expands to_PyList_CAST(op)->ob_item[index] = value, so it writes throughNULLplus an offset.PyList_SetItem(py_lines, i, py_line)starts withPyList_Check(op), which evaluatesPy_TYPE(op)and dereferencesNULL.The trigger is an allocation failure, so this is not reachable from ordinary code, but the failure mode is a segfault rather than a recoverable exception.
Both loops
return NULLmid-iteration without releasing the list they already own. InDiffHunk_lines__get__there are two such paths (git_patch_get_line_in_hunkfailing, andwrap_diff_linereturningNULL); inPatch_hunks__get__there is one.Suggested fix
Check the allocation and release the list on every early return:
Apply the equivalent shape to
Patch_hunks__get__.