Skip to content

mm_new/mm_new_nan: three unchecked malloc calls — segfault on OOM #526

@devdanzin

Description

@devdanzin

mm_new and mm_new_nan each call malloc three times in sequence without checking any result for NULL. The second call dereferences the first result (mm->nodes = malloc(...) where mm could be NULL), and subsequent lines dereference mm->nodes.

Additionally, mm_free does not check for NULL before dereferencing its argument. This compounds with the move_median window==1 fast path, which calls mm_free(mm) before checking if mm is NULL.

File(s): move_median/move_median.c:62-64 (mm_new), move_median/move_median.c:170-172 (mm_new_nan), move_template.c:564-566 (mm_free on NULL)

Suggested fix for mm_new:

// Before:
mm_handle *mm = malloc(sizeof(mm_handle));
mm->nodes = malloc(window * sizeof(mm_node*));
mm->node_data = malloc(window * sizeof(mm_node));

// After:
mm_handle *mm = malloc(sizeof(mm_handle));
if (mm == NULL) return NULL;
mm->nodes = malloc(window * sizeof(mm_node*));
mm->node_data = malloc(window * sizeof(mm_node));
if (mm->nodes == NULL || mm->node_data == NULL) {
    free(mm->node_data);
    free(mm->nodes);
    free(mm);
    return NULL;
}

Same pattern for mm_new_nan. For the window==1 path, move the NULL check before mm_free:

if (mm == NULL) {
    MEMORY_ERR("Could not allocate memory for move_median");
    Py_DECREF(y);
    return NULL;
}
if (window == 1) {
    mm_free(mm);
    return PyArray_Copy(a);
}

See #518 for the complete report.

Found using cext-review-toolkit.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No fields configured for Bug.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions