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
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions