Skip to content

PyObject_RichCompareBool error treated as truthy in range_handler + safe_richcompare swallows KeyboardInterrupt #256

@devdanzin

Description

@devdanzin

Two related comparison error-handling bugs:

1. range_handler error-as-truthy (validatebehavior.cpp:884,894)

PyObject_RichCompareBool returns -1 on error, 0 for false, 1 for true. The code uses if(PyObject_RichCompareBool(...)) which treats -1 (error) as truthy, entering the branch and calling PyErr_Format which clobbers the original exception with a misleading ValueError.

Reproducer:

import gc; gc.disable()
import os
from atom.api import Atom, Range

class BadInt(int):
    def __gt__(self, other):
        raise TypeError("comparison bomb")
    def __lt__(self, other):
        raise TypeError("comparison bomb")

class MyAtom(Atom):
    y = Range(low=0, high=100)

a = MyAtom()
a.y = 50  # works
try:
    a.y = BadInt(50)
except TypeError as e:
    print(f"TypeError (correct): {e}")
except ValueError as e:
    print(f"ValueError (CLOBBERED - BUG): {e}")
os._exit(0)
# Output: ValueError (CLOBBERED): range value for 'y' of 'MyAtom' too small

Fix: check < 0 for error before checking truthy:

int cmp = PyObject_RichCompareBool( low, newvalue, Py_GT );
if( cmp < 0 )
    return 0;  // propagate the exception
if( cmp )
    return PyErr_Format(...);

2. safe_richcompare unguarded PyErr_Clear (utils.h:112-113)

safe_richcompare is used as an STL comparator in SortedMap. When PyObject_RichCompareBool fails, it clears ALL pending exceptions without checking the type — MemoryError and KeyboardInterrupt are silently swallowed.

Reproducer:

import gc; gc.disable()
import os
from atom.datastructures.sortedmap import sortedmap

class ComparisonBomb:
    def __lt__(self, other):
        raise KeyboardInterrupt("Ctrl-C during comparison")
    def __gt__(self, other):
        raise KeyboardInterrupt("Ctrl-C during comparison")
    def __eq__(self, other):
        raise KeyboardInterrupt("Ctrl-C during comparison")

m = sortedmap()
m[ComparisonBomb()] = "first"
try:
    m[ComparisonBomb()] = "second"
    print(f"KeyboardInterrupt SWALLOWED (BUG) - len={len(m)}")
except KeyboardInterrupt:
    print("KeyboardInterrupt propagated (correct)")
os._exit(0)
# Output: KeyboardInterrupt SWALLOWED (BUG) - len=2

Fix: guard the clear with PyErr_ExceptionMatches(PyExc_TypeError):

if( PyErr_Occurred() )
{
    if( !PyErr_ExceptionMatches( PyExc_TypeError ) )
        return false;  // or propagate somehow
    PyErr_Clear();
}

Found by cext-review-toolkit.

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