Skip to content

Commit

Permalink
Fix a soundness bug with PyClassInitializer
Browse files Browse the repository at this point in the history
From now you cannot initialize a `PyClassInitializer<SubClass>` with `PyClassInitializer::from(Py<BaseClass>).add_subclass(SubClass)`.

This was out of bounds write. Now it panics. See details at #4452.
  • Loading branch information
ChayimFriedman2 committed Aug 19, 2024
1 parent 4211c57 commit 1749f4c
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 0 deletions.
1 change: 1 addition & 0 deletions newsfragments/4454.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix a soundness bug with `PyClassInitializer`: from now you cannot initialize a `PyClassInitializer<SubClass>` with `PyClassInitializer::from(Py<BaseClass>).add_subclass(SubClass)`.
6 changes: 6 additions & 0 deletions src/pyclass_init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,11 +197,17 @@ impl<T: PyClass> PyClassInitializer<T> {
/// })
/// }
/// ```
#[track_caller]
#[inline]
pub fn add_subclass<S>(self, subclass_value: S) -> PyClassInitializer<S>
where
S: PyClass<BaseType = T>,
S::BaseType: PyClassBaseType<Initializer = Self>,
{
if matches!(self.0, PyClassInitializerImpl::Existing { .. }) {
// This is unsound; see https://github.com/PyO3/pyo3/issues/4452.
panic!("you cannot add a subclass to an existing value");
}
PyClassInitializer::new(subclass_value, self)
}

Expand Down
20 changes: 20 additions & 0 deletions tests/add_subclass_to_py.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
//! See https://github.com/PyO3/pyo3/issues/4452.
use pyo3::prelude::*;

#[pyclass(subclass)]
struct BaseClass {}

#[pyclass(extends=BaseClass)]
struct SubClass {
_data: i32,
}

#[test]
#[should_panic]
fn add_subclass_to_py_is_unsound() {
Python::with_gil(|py| {
let base = Py::new(py, BaseClass {}).unwrap();
let _subclass = PyClassInitializer::from(base).add_subclass(SubClass { _data: 42 });
});
}

0 comments on commit 1749f4c

Please sign in to comment.