Skip to content

_csv: _set_str() missing NULL check after PyUnicode_DecodeASCII() #145978

Closed as duplicate

Description

@raminfp

Bug report

Bug description:

In Modules/_csv.c, _set_str() does not check the return value of PyUnicode_DecodeASCII() when using the default value. If the allocation fails (OOM), *target is set to NULL and the function returns 0 (success).

https://github.com/python/cpython/blob/main/Modules/_csv.c#L315-L329

static int
_set_str(const char *name, PyObject **target, PyObject *src, const char *dflt)
{
    if (src == NULL)
        *target = PyUnicode_DecodeASCII(dflt, strlen(dflt), NULL);
        // ^^^ no NULL check 
    else {
        ...
    }
    return 0; // Returns 0 even if allocation failed
}

This function is called for lineterminator:

DIASET(_set_str, "lineterminator", &self->lineterminator, lineterminator, "\r\n");

If allocation fails, self->lineterminator is NULL. Subsequent calls to PyUnicode_GET_LENGTH(dialect->lineterminator) (e.g. in join_append_data at line 1183 or join_append_lineterminator at line 1299) will dereference NULL.

Fix

static int
_set_str(const char *name, PyObject **target, PyObject *src, const char *dflt)
{
    if (src == NULL) {
        *target = PyUnicode_DecodeASCII(dflt, strlen(dflt), NULL);
        if (*target == NULL)
            return -1;
    }
    else {
        if (!PyUnicode_Check(src)) {
            PyErr_Format(PyExc_TypeError,
                         "\"%s\" must be a string, not %T", name, src);
            return -1;
        }
        Py_XSETREF(*target, Py_NewRef(src));
    }
    return 0;
}

CPython versions tested on:

CPython main branch

Operating systems tested on:

Linux

Activity

  1. StanFromIreland commented on Mar 15, 2026

    @StanFromIreland
    Member

    Would you like to send a PR?

  2. raminfp commented on Mar 18, 2026

    @raminfp
    ContributorAuthor

    @StanFromIreland Yes, I must have just gotten online.

  3. raminfp commented on Mar 18, 2026

    @raminfp
    ContributorAuthor

    I checked and this issue was reported again after I reported it. #146093

  4. raminfp commented on Mar 18, 2026

    @raminfp
    ContributorAuthor

    Issue fixed by #146113. @vstinner

  5. raminfp commented on Mar 18, 2026

    @raminfp
    ContributorAuthor

    This report of mine was open and I don't know how you filed it again. @devdanzin

    CC: @StanFromIreland

  6. vstinner commented on Mar 18, 2026

    @vstinner
    Member

    This report of mine was open and I don't know how you filed it again. @devdanzin

    @devdanzin wrote that he found the issue using cpython-review-toolkit with Claude Opus 4.6. How did you find this bug? Did you also use a LLM?

  7. raminfp commented on Mar 18, 2026

    @raminfp
    ContributorAuthor

    No, I checked the method returns with manual code review and saw it.

  8. vstinner commented on Mar 18, 2026

    @vstinner
    Member

    Well, it's just a coincidence that @devdanzin and you found the bug a few days apart using different technics. For me, the important part is that the bug has been fixed :-) Thanks for your report @raminfp anyway!

  9. raminfp commented on Mar 18, 2026

    @raminfp
    ContributorAuthor

    Please, yes, the bug is fixed, and thank you for your good work. @devdanzin You started a good project for code review.

  10. devdanzin commented on Mar 19, 2026

    @devdanzin
    Member

    Sorry @raminfp, I should have checked for existing issues before creating one! This is a great issue, nice description and fix, and you were ready to create a PR too, so my mistake got in the way of our best practices and of your participation. Truly sorry, I hope not to do this kind of disservice in the future.

    And thank you for your contribution and understanding.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    extension-modulesC modules in the Modules dirtype-bugAn unexpected behavior, bug, or error

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions