Fix loophole in soundness of `__private_get_type_id__`

The `downcast_ref` and `downcast_mut` methods rely on
the `TypeId` returned from `__private_get_type_id__` for soundness.
To prevent users from overriding `__private_get_type_id__`, it returns
a tuple containing a type with a private constructor (`PrivateHelper`),
ensuring that any user-provided implementation cannot actually return.

However, there's a loophole - safe code could obtain an instance of
`PrivateHelper` by calling `__private_get_type_id__` on an *existing*
implementation, and then returning that `PrivateHelper` instance from
their own implementation. While this is incredibly contrived, and
could never happen by accident, it's still technically a soundness hole.

To fix this, `__private__get_type_id__` is changed to also take in a
`PrivateHelper` as a parameter. Now, safe code cannot use this method to
obtain a `PrivateHelper`, since a `PrivateHelper` would already need to
be available in order to call the method in the first place.
This commit is contained in:
Aaron Hill 2021-05-04 18:07:45 -04:00
parent c17662fe39
commit 7940292432
No known key found for this signature in database
GPG Key ID: B4087E510E98B164
1 changed files with 14 additions and 3 deletions

View File

@ -15,8 +15,15 @@ macro_rules! downcast_get_type_id {
/// making it impossible for safe code to construct outside of
/// this module. This ensures that safe code cannot violate
/// type-safety by implementing this method.
///
/// We also take `PrivateHelper` as a parameter, to ensure that
/// safe code cannot obtain a `PrivateHelper` instance by
/// delegating to an existing implementation of `__private_get_type_id__`
#[doc(hidden)]
fn __private_get_type_id__(&self) -> (std::any::TypeId, PrivateHelper)
fn __private_get_type_id__(
&self,
_: PrivateHelper,
) -> (std::any::TypeId, PrivateHelper)
where
Self: 'static,
{
@ -39,7 +46,9 @@ macro_rules! downcast {
impl dyn $name + 'static {
/// Downcasts generic body to a specific type.
pub fn downcast_ref<T: $name + 'static>(&self) -> Option<&T> {
if self.__private_get_type_id__().0 == std::any::TypeId::of::<T>() {
if self.__private_get_type_id__(PrivateHelper(())).0
== std::any::TypeId::of::<T>()
{
// SAFETY: external crates cannot override the default
// implementation of `__private_get_type_id__`, since
// it requires returning a private type. We can therefore
@ -53,7 +62,9 @@ macro_rules! downcast {
/// Downcasts a generic body to a mutable specific type.
pub fn downcast_mut<T: $name + 'static>(&mut self) -> Option<&mut T> {
if self.__private_get_type_id__().0 == std::any::TypeId::of::<T>() {
if self.__private_get_type_id__(PrivateHelper(())).0
== std::any::TypeId::of::<T>()
{
// SAFETY: external crates cannot override the default
// implementation of `__private_get_type_id__`, since
// it requires returning a private type. We can therefore