ctraits Extension Analysis Report
- Dominant language
- Python
- Stars
- 462
- Forks
- 90
- PR merge metrics
- No merged PRs in 30d
Description
I've run [cext-review-toolkit](https://github.com/devdanzin/cext-review-toolkit) on traits and got a report that includes the issues below:
**1. Use-after-free in `setattr_delegate` — borrowed dict ref used after DECREF**
- **Location**: `ctraits.c:2587-2597`
- **Impact**: `has_traits_getattro` returns new ref -> immediate `Py_DECREF` -> `delegate` is dangling -> subsequent use is use-after-free. The analogous code in `getattr_delegate` and `_has_traits_trait` correctly keeps the reference alive.
**2. Spurious `Py_DECREF(name)` on borrowed parameter in `setattr_trait`**
- **Location**: `ctraits.c:2526`
- **Impact**: `name` is borrowed from `tp_setattro` caller. DECREF on `PyDict_SetItem` error path corrupts refcount, causing use-after-free in the caller.
**3. Unbounded pickle indices in `_trait_setstate` — potential arbitrary code execution**
- **Location**: `ctraits.c:4923-4929`
- **Impact**: Indices from untrusted pickle data index into function pointer arrays without bounds checks. Out-of-bounds read yields garbage function pointer -> arbitrary code execution when called. Compare with `trait_new` which correctly bounds-checks.
**4. `_trait_setstate` backward compat: NULL deref + double-INCREF leak**
- **Location**: `ctraits.c:4939-4950`
- **Impact**: `PyObject_GetAttrString` can return NULL -> `Py_INCREF(NULL)` crashes. When it succeeds, the new reference + unconditional `Py_INCREF` = double reference that leaks.
**5. `trait_clone` leaks old references when called on existing objects**
- **Location**: `ctraits.c:4773-4794`
- **Impact**: Overwrites 6 `PyObject*` fields (handler, py_validate, py_post_setattr, default_value, delegate_name, delegate_prefix) without XDECREF'ing old values. Leaks up to 6 refs per `clone()` call. Compare with `_trait_set_default_value` which uses the correct INCREF-new/XDECREF-old pattern.
**6. `_trait_set_property` leaks old `delegate_name`, `delegate_prefix`, `py_validate`**
- **Location**: `ctraits.c:4758-4763`
- **Impact**: Same class as Finding 5. Overwrites 3 fields without releasing old values.
**7. `_trait_delegate` leaks old `delegate_name` and `delegate_prefix`**
- **Location**: `ctraits.c:4616-4627`
- **Impact**: Same class as Findings 5-6.
**8. `has_traits_new` leaks `obj` on all 3 error paths**
- **Location**: `ctraits.c:694-710`
- **Impact**: `tp_new` succeeds but validation fails -> 3 `return NULL` without `Py_DECREF(obj)`.
**9. `get_trait`: unchecked `PyType_GenericAlloc` + 2 leak paths for `itrait`**
- **Location**: `ctraits.c:950-978`
- **Impact**: NULL deref if alloc fails; `itrait` leaked if `PyList_New` or `PyDict_SetItem` fails.
**10. `has_traits_init` passes NULL `PyDict_GetItem` result to `PyMapping_Size`**
- **Location**: `ctraits.c:729-731`
- **Impact**: `PyDict_GetItem` returns NULL -> `PyMapping_Size(NULL)` -> crash.
**11. Unguarded `PyErr_Clear()` calls in `validate_trait_complex` (4 sites)**
- **Location**: `ctraits.c:4065,4072,4132,4142`
- **Impact**: Cases 5 (enum), 6 (mapped), 12 (castable), 13 (function validator) all swallow `MemoryError`/`KeyboardInterrupt`.
**12. Unguarded `PyErr_Clear()` in `delegate_attr_name_class_name`**
- **Location**: `ctraits.c:4584`
- **Impact**: `MemoryError` from `PyObject_GetAttr` silently swallowed, returns wrong delegate name.
**13. `get_prefix_trait` ignores `PyDict_SetItem` failure + dereferences potential NULL**
- **Location**: `ctraits.c:630,637`
- **Impact**: `PyDict_SetItem` failure ignored (exception clobbered by subsequent calls); `get_trait` NULL return passed to `Py_DECREF`.
**14. `setattr_property0` leaks `args` tuple on call failure**
- **Location**: `ctraits.c:2668-2675`
**15. `_trait_getstate` stores unchecked `PyLong_FromLong` NULLs in tuple**
- **Location**: `ctraits.c:4867-4897`
**16. `PyModule_AddObject` error paths leak type refs + module object**
- **Location**: `ctraits.c:5719-5735`
**17. Unchecked `PyUnicode_FromString` in init (5 sites)**
- **Location**: `ctraits.c:5738-5750`
**18. Unchecked `PyType_GenericNew` in `trait_new`**
- **Location**: `ctraits.c:2968-2970`
**19. `validate_trait_tuple_check` leaks `aitem` when tuple allocation fails**
- **Location**: `ctraits.c:3697-3700`
**20. `setattr_delegate` recursion error leaks `daname`**
- **Location**: `ctraits.c:2646-2648`
The main reproducers are:
## Reproducer 1: Segfault from `setattr_delegate` Use-After-Free via Property Delegate
**Bug**: `setattr_delegate` calls `has_traits_getattro` to get the delegate object (returns new ref with refcount 1), then immediately calls `Py_DECREF` (refcount 0, object freed). The freed `delegate` pointer is then used for `PyHasTraits_Check`, `delegate_attr_name`, and dict lookups — all use-after-free. Triggered when a delegate is a property that creates a fresh object on each access.
**File**: `ctraits.c:2587-2597`
```python
from traits.api import HasTraits, Int, DelegatesTo, Property
class Inner(HasTraits):
value = Int(42)
class Outer(HasTraits):
inner = Property(lambda self: Inner())
value = DelegatesTo("inner")
Outer().value = 99 # Segmentation fault (core dumped)
```
**Output**:
```
Segmentation fault (core dumped)
```
---
## Reproducer 2: Segfault from `clone()` with Handler Reassignment
**Bug**: `trait_clone` overwrites `handler` and 5 other `PyObject*` fields without XDECREF'ing old values. When called repeatedly with different handlers, the old handler's refcount becomes corrupt, leading to a crash.
**File**: `ctraits.c:4773-4794`
```python
from traits.ctraits import cTrait
target = cTrait(0)
for i in range(5000):
source = cTrait(0)
source.handler = lambda: i
target.clone(source)
# Segmentation fault (core dumped)
```
**Output**:
```
Segmentation fault (core dumped)
```
---
## Reproducer 3: Segfault from Unchecked Allocation Under OOM (`get_trait`)
**Bug**: `get_trait` calls `PyType_GenericAlloc` (line 950) and `PyList_New` (line 958) without NULL checks. When allocation fails under memory pressure, the NULL pointer is dereferenced, crashing the interpreter. Triggered using `_testcapi.set_nomemory` to inject OOM at successive allocation points.
**File**: `ctraits.c:950-960`
```python
import _testcapi
from traits.api import HasTraits, Int, on_trait_change
class Obj(HasTraits):
x = Int()
@on_trait_change("x")
def _x_changed(self, new):
pass
for n in range(1, 500):
_testcapi.set_nomemory(n, 0)
try:
obj = Obj()
obj.x = 42
_testcapi.remove_mem_hooks()
except MemoryError:
_testcapi.remove_mem_hooks()
except:
_testcapi.remove_mem_hooks()
# Segmentation fault (core dumped)
```
**Output**:
```
Segmentation fault (core dumped)
```
Please consult https://gist.github.com/devdanzin/c997faaee7556ffc5cb991deca553966 for the full report which includes more issues and reproducers.
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.