From 0aff6c8b6d61f90535c72f27177775dd010aa78c Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Tue, 15 Apr 2025 09:04:02 -0700 Subject: [PATCH 1/2] Prevent over-deep type hierarchies in wasm-smith Currently wasm-smith can produce invalid GC modules because there's no limit placed on subtyping depth. This commit keeps track of subtyping depth and appropriately manages the `can_subtype` list as a result. --- crates/wasm-smith/src/core.rs | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/crates/wasm-smith/src/core.rs b/crates/wasm-smith/src/core.rs index 1e01785e8b..972f2be8f8 100644 --- a/crates/wasm-smith/src/core.rs +++ b/crates/wasm-smith/src/core.rs @@ -592,7 +592,24 @@ impl Module { list.push(index); if !ty.is_final { - self.can_subtype.push(index); + // Calculate the recursive depth of this type, and if it's beneath a + // threshold then allow future types to subtype this one. Otherwise + // this can no longer be subtyped so despite this not being final + // don't add it to the `can_subtype` list. + // + // Note that this limit is intentinally a bit less than the + // wasm-defined maximum of 63. + const MAX_SUBTYPING_DEPTH: u32 = 60; + + let mut depth = 1; + let mut cur = ty.supertype; + while let Some(parent) = cur { + depth += 1; + cur = self.types[parent as usize].supertype; + } + if depth < MAX_SUBTYPING_DEPTH { + self.can_subtype.push(index); + } } self.types.push(ty); From 86471762710257fc549380ba58c950e59791be26 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Tue, 15 Apr 2025 10:33:54 -0700 Subject: [PATCH 2/2] Review comments --- crates/wasm-smith/src/core.rs | 37 +++++++++++++++++------------------ 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/crates/wasm-smith/src/core.rs b/crates/wasm-smith/src/core.rs index 972f2be8f8..1467cfa5e2 100644 --- a/crates/wasm-smith/src/core.rs +++ b/crates/wasm-smith/src/core.rs @@ -260,6 +260,9 @@ pub(crate) struct SubType { pub(crate) is_final: bool, pub(crate) supertype: Option, pub(crate) composite_type: CompositeType, + /// How "deep" this subtype's supertype hierarchy is. The base case is 1 and + /// if `supertype` is present it's `1 + supertype.depth`. + depth: u32, } impl SubType { @@ -591,25 +594,16 @@ impl Module { }; list.push(index); - if !ty.is_final { - // Calculate the recursive depth of this type, and if it's beneath a - // threshold then allow future types to subtype this one. Otherwise - // this can no longer be subtyped so despite this not being final - // don't add it to the `can_subtype` list. - // - // Note that this limit is intentinally a bit less than the - // wasm-defined maximum of 63. - const MAX_SUBTYPING_DEPTH: u32 = 60; - - let mut depth = 1; - let mut cur = ty.supertype; - while let Some(parent) = cur { - depth += 1; - cur = self.types[parent as usize].supertype; - } - if depth < MAX_SUBTYPING_DEPTH { - self.can_subtype.push(index); - } + // Calculate the recursive depth of this type, and if it's beneath a + // threshold then allow future types to subtype this one. Otherwise this + // can no longer be subtyped so despite this not being final don't add + // it to the `can_subtype` list. + // + // Note that this limit is intentinally a bit less than the wasm-defined + // maximum of 63. + const MAX_SUBTYPING_DEPTH: u32 = 60; + if !ty.is_final && ty.depth < MAX_SUBTYPING_DEPTH { + self.can_subtype.push(index); } self.types.push(ty); @@ -699,6 +693,7 @@ impl Module { is_final: true, supertype: None, composite_type, + depth: 1, }); } @@ -709,6 +704,7 @@ impl Module { is_final: u.arbitrary()?, supertype: None, composite_type: self.arbitrary_composite_type(u)?, + depth: 1, }) } } @@ -735,6 +731,7 @@ impl Module { is_final: u.arbitrary()?, supertype: Some(supertype), composite_type, + depth: 1 + self.types[supertype as usize].depth, }) } @@ -1381,6 +1378,7 @@ impl Module { is_final: true, supertype: None, composite_type: CompositeType::new_func(Rc::clone(&func_type), shared), + depth: 1, }); new_index } @@ -1965,6 +1963,7 @@ impl Module { let type_index = self.add_type(SubType { is_final: true, supertype: None, + depth: 1, composite_type: CompositeType::new_func( Rc::clone(&new_type), subtype.composite_type.shared,