Return value of <ExceptionGroup class>.split has insufficient checks leading to a type confusion bug

[Nico-Posada]

Crash report

What happened?

The following code has checks to make sure the return value is a tuple and of size 2, but only in asserts which means that these checks wont happen on a non-debug build.

cpython/Python/ceval.c

Lines 2093 to 2101 in b92f101

	PyObject *pair = PyObject_CallMethod(exc_value, "split", "(O)",
	match_type);
	if (pair == NULL) {
	return -1;
	}
	assert(PyTuple_CheckExact(pair));
	assert(PyTuple_GET_SIZE(pair) == 2);
	*match = Py_NewRef(PyTuple_GET_ITEM(pair, 0));
	*rest = Py_NewRef(PyTuple_GET_ITEM(pair, 1));

So you can create an ExceptionGroup subclass with a custom split function that doesnt return a tuple, and it will try to interpret that object as a tuple.

PoC

class Evil(BaseExceptionGroup):
    def split(self, *args):
        return "NOT A TUPLE!"

print("Running...")
try:
    raise Evil("wow!", [Exception()])
except* Exception:
    pass

print("program should crash before reaching this")

Output

Running...
Segmentation fault (core dumped)

CPython versions tested on:

3.11, 3.12, 3.13

Operating systems tested on:

Linux, Windows

Output from running 'python -VV' on the command line:

No response

Linked PRs

• gh-128049: Fix type confusion bug with the return value of a custom ExceptionGroup split function #128079
• [3.13] gh-128049: Fix type confusion bug with the return value of a custom ExceptionGroup split function (GH-128079) #128139
• [3.12] gh-128049: Fix type confusion bug with the return value of a custom ExceptionGroup split function (GH-128079) #128140

[tomasr8]

Nice find! Raising a TypeError seems more appropriate here, would you like to send a PR?

[Nico-Posada]

Sure

[iritkatriel]

I agree we should do something here, but raising a new exception while the interpreter is handling an exception from the program is not ideal if it swallows the user's exception.

So we need to think that through.

[Nico-Posada]

With the change I implemented, the new output is

Running...
  + Exception Group Traceback (most recent call last):
  |   File "/tmp/test.py", line 7, in <module>
  |     raise Evil("wow!", [Exception()])
  | Evil: wow! (1 sub-exception)
  +-+---------------- 1 ----------------
    | Exception
    +------------------------------------

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/tmp/test.py", line 8, in <module>
    except* Exception:
        pass
TypeError: Evil.split must return a 2-tuple

[iritkatriel]

Could you update the test to check that the original exception is chained?

[Nico-Posada]

hmm, if I update the test to the following, then it final print statement executes

class Evil(BaseExceptionGroup):
    def split(self, *args):
        return "NOT A TUPLE!"

print("Running...")

try:
    try:
        raise Evil("wow!", [Exception()])
    except* Exception:
        pass
except TypeError:
    pass

print("done")

But if the original exception is dependent on the ExceptionGroup split function executing correctly, how else can we go about it?

[iritkatriel]

hmm, if I update the test to the following, then it final print statement executes

On your branch I would expect it to work. except *Exception will catch a TypeError.

But if the original exception is dependent on the ExceptionGroup split function executing correctly, how else can we go about it?

PyErr_FormatUnraisable is also an option.

[iritkatriel]

To clarify, the alternative suggestion is to treat anything other than a 2-tuple as a no-match, and raise a TypeError via PyErr_FormatUnraisable.

[iritkatriel]

Though, chaining would be consistent with this:

>>> def t(): return ValueError
... 
>>> try:
...     raise ValueError(42)
... except t() as e:
...     print(e)
...     
42
>>> 
>>> def t(): raise TypeError
... 
>>> try:
...     raise ValueError(42)
... except t() as e:
...     print(e)
...     
Traceback (most recent call last):
  File "<python-input-12>", line 2, in <module>
    raise ValueError(42)
ValueError: 42

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<python-input-12>", line 3, in <module>
    except t() as e:
           ~^^
  File "<python-input-11>", line 1, in t
    def t(): raise TypeError
             ^^^^^^^^^^^^^^^
TypeError
>>> 

[iritkatriel]

For the test, you need to use a different type so that you're not catching everything. Something like this:

class Evil(BaseExceptionGroup):
    def split(self, *args):
        return "NOT A TUPLE!"

print("Running...")

try:
    try:
        raise Evil("wow!", [OSError(), ValueError()])
    except* ValueError:
        pass
    except* OSError:
        pass
except TypeError as e:
    assert that e is what we expect
    assert that e.__context__ is the raised exception with `ValueErrors` split out

print("done")

[Nico-Posada]

I've never worked with FormatUnraisable before, is this the correct way to use it?

/* ... */

if (_PyBaseExceptionGroup_Check(exc_value)) {
        PyObject *pair = PyObject_CallMethod(exc_value, "split", "(O)",
                                             match_type);
        if (pair == NULL) {
            return -1;
        }

        if (!PyTuple_CheckExact(pair) || PyTuple_Size(pair) != 2) {
            PyErr_Format(PyExc_TypeError, 
                        "%s.split must return a 2-tuple",
                        Py_TYPE(exc_value)->tp_name);
            PyErr_FormatUnraisable("Exception ignored on calling %s.split",
                                    Py_TYPE(exc_value)->tp_name);
            Py_DECREF(pair);
            goto no_match;
        }

        *match = Py_NewRef(PyTuple_GET_ITEM(pair, 0));
        *rest = Py_NewRef(PyTuple_GET_ITEM(pair, 1));
        Py_DECREF(pair);
        return 0;
    }

no_match:
    *match = Py_NewRef(Py_None);
    *rest = Py_NewRef(exc_value);
    return 0;
}

If I don't put the PyErr_Format before it, I get an assertion failure of

python.exe: Python/errors.c:1600: format_unraisable_v: Assertion `exc_type != NULL' failed.

With the changes above, I get this output

Running...
Exception ignored on calling Evil.split:
Traceback (most recent call last):
  File "/tmp/test.py", line 9, in <module>
    except* Exception:
TypeError: Evil.split must return a 2-tuple
  + Exception Group Traceback (most recent call last):
  |   File "/tmp/test.py", line 8, in <module>
  |     raise Evil("wow!", [Exception()])
  | ExceptionGroup: wow! (1 sub-exception)
  +-+---------------- 1 ----------------
    | Exception
    +------------------------------------

[iritkatriel]

I've never worked with FormatUnraisable before, is this the correct way to use it?

That looks right. But see my previous comment - I think it will be consistent with the other case to go with your first suggestion.