Skip to content

NULL dereference in DiffHunk.lines and Patch.hunks when list allocation fails #1479

Description

@K-ANOY

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__.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions