Jan 31, 2025 · Oct 29, 2024 · Oct 29, 2024 · Oct 29, 2024 · Oct 29, 2024 · Oct 29, 2024
diff --git a/Misc/NEWS.d/next/Security/2024-10-29-09-15-10.gh-issue-126108.eTIjHY.rst b/Misc/NEWS.d/next/Security/2024-10-29-09-15-10.gh-issue-126108.eTIjHY.rst
 Fix a possible ``NULL`` pointer dereference in :c:func:`!PySys_AddWarnOptionUnicode`.
diff --git a/Python/sysmodule.c b/Python/sysmodule.c
 static int
 _PySys_AddWarnOptionWithError(PyThreadState *tstate, PyObject *option)
 {
    assert(tstate != NULL);
    PyObject *warnoptions = get_warnoptions(tstate);
    if (warnoptions == NULL) {
        return -1;
 PySys_AddWarnOptionUnicode(PyObject *option)
 {
    PyThreadState *tstate = _PyThreadState_GET();
    if (_PySys_AddWarnOptionWithError(tstate, option) < 0) {
    if (tstate  &&_PySys_AddWarnOptionWithError(tstate, option) < 0) {
        /* No return value, therefore clear error state if possible */
        if (tstate) {
            _PyErr_Clear(tstate);
        }
        _PyErr_Clear(tstate);
    }
 }
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		Fix a possible ``NULL`` pointer dereference in :c:func:`!PySys_AddWarnOptionUnicode`.
Original file line number	Diff line number	Diff line change
Expand Up		@@ -2838,6 +2838,7 @@ PySys_ResetWarnOptions(void)
		static int
		_PySys_AddWarnOptionWithError(PyThreadState tstate, PyObject option)
		{
		assert(tstate != NULL);
		PyObject *warnoptions = get_warnoptions(tstate);
		if (warnoptions == NULL) {
		return -1;
Expand All		@@ -2853,11 +2854,9 @@ PyAPI_FUNC(void)
		PySys_AddWarnOptionUnicode(PyObject *option)
		{
		PyThreadState *tstate = _PyThreadState_GET();
federicovalenso marked this conversation as resolved. Show resolvedHide resolved
		if (_PySys_AddWarnOptionWithError(tstate, option) < 0) {
		if (tstate &&_PySys_AddWarnOptionWithError(tstate, option) < 0) {
federicovalenso marked this conversation as resolved. OutdatedShow resolvedHide resolved
		/* No return value, therefore clear error state if possible */
		if (tstate) {
picnixz marked this conversation as resolved. Show resolvedHide resolved
		_PyErr_Clear(tstate);
		}
		_PyErr_Clear(tstate);
Copy link Member picnixzOct 30, 2024 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. I assume that there's no way to make`tstate` NULL in`_PySys_AddWarnOptionWithError`. Copy link Member sobolevnOct 30, 2024 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. I agree,`assert` is the way to go here. We should not call these functions in places where`tstate` can be`NULL` (while interpreter startup / shutdown). Copy link Member picnixzOct 30, 2024 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. Would it actually make sense to have`_PyErr_Clear` do this automatically? Copy link Member ZeroIntensityOct 30, 2024 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. Ok, in that case, I would like`PySys_AddWarnOptionUnicode` to fail with a fatal error if called without the GIL. No-oping because of a null thread state isn't common at all in the C API. Copy link ContributorAuthor federicovalensoOct 31, 2024 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. Should I do some other changes or leave as is? Copy link Member picnixzOct 31, 2024• edited Loading Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. Should we also do it in`_PySys_AddWarnOptionWithError`? WDYT@ZeroIntensity? Copy link Member ZeroIntensityOct 31, 2024 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. I think`assert(tstate != NULL)` is fine for the private API. picnixz reacted with thumbs up emoji
		}
		}

Expand Down