From 82ba2a073f0197a2cdad164e3ca633a88edee065 Mon Sep 17 00:00:00 2001 From: overlookmotel <557937+overlookmotel@users.noreply.github.com> Date: Fri, 26 Jul 2024 00:14:44 +0000 Subject: [PATCH] fix(syntax): fix unsound use of `NonZeroU32` (#4466) `NonZeroU32::new_unchecked(idx as u32 + 1)` is unsound because if `idx == u32::MAX`, `idx + 1` wraps around back to zero. So unfortunately we need to use the checked version `NonZeroU32::new(idx as u32 + 1).unwrap()` to avoid UB in this edge case. --- crates/oxc_syntax/src/reference.rs | 6 +++--- crates/oxc_syntax/src/symbol.rs | 12 ++++++------ 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/crates/oxc_syntax/src/reference.rs b/crates/oxc_syntax/src/reference.rs index 61a72bcb9b54b..45db818aa74fa 100644 --- a/crates/oxc_syntax/src/reference.rs +++ b/crates/oxc_syntax/src/reference.rs @@ -13,9 +13,9 @@ pub struct ReferenceId(NonZeroU32); impl Idx for ReferenceId { #[allow(clippy::cast_possible_truncation)] fn from_usize(idx: usize) -> Self { - // SAFETY: + 1 is always non-zero. - - unsafe { Self(NonZeroU32::new_unchecked(idx as u32 + 1)) } + // NB: We can't use `NonZeroU32::new_unchecked(idx as u32 + 1)` + // because if `idx == u32::MAX`, `+ 1` would make it wrap around back to 0 + Self(NonZeroU32::new(idx as u32 + 1).unwrap()) } fn index(self) -> usize { diff --git a/crates/oxc_syntax/src/symbol.rs b/crates/oxc_syntax/src/symbol.rs index 6688e84ec7171..5274d5b33b74c 100644 --- a/crates/oxc_syntax/src/symbol.rs +++ b/crates/oxc_syntax/src/symbol.rs @@ -13,9 +13,9 @@ pub struct SymbolId(NonZeroU32); impl Idx for SymbolId { #[allow(clippy::cast_possible_truncation)] fn from_usize(idx: usize) -> Self { - // SAFETY: + 1 is always non-zero. - - unsafe { Self(NonZeroU32::new_unchecked(idx as u32 + 1)) } + // NB: We can't use `NonZeroU32::new_unchecked(idx as u32 + 1)` + // because if `idx == u32::MAX`, `+ 1` would make it wrap around back to 0 + Self(NonZeroU32::new(idx as u32 + 1).unwrap()) } fn index(self) -> usize { @@ -30,9 +30,9 @@ pub struct RedeclarationId(NonZeroU32); impl Idx for RedeclarationId { #[allow(clippy::cast_possible_truncation)] fn from_usize(idx: usize) -> Self { - // SAFETY: + 1 is always non-zero. - - unsafe { Self(NonZeroU32::new_unchecked(idx as u32 + 1)) } + // NB: We can't use `NonZeroU32::new_unchecked(idx as u32 + 1)` + // because if `idx == u32::MAX`, `+ 1` would make it wrap around back to 0 + Self(NonZeroU32::new(idx as u32 + 1).unwrap()) } fn index(self) -> usize {