What the developer wrote
Authored by Andrew Poelstra
remove some pointer casts
I grepped the codebase for `as \*` and found a few places where we were
doing explicit pointer casts but we could've been using std methods. The
methods are safer because they don't allow changing mutability (unless
we use cast_const or cast_mut, which don't allow changing the type).
There are a few instances remaining where we cast to pointers to unsized
types. These are actually fat raw pointers and they don't have std
methods. Now they stand out because of the use of `as *`.
This commit might require a bit of thought to review, and it turns out
it has nothing to do with the MSRV bump (which gave us cast_const and
cast_mut, but actually we don't use those, thankfully..).
As a bit of trivia, though, the new cast_mut and cast_const methods were
originally proposed and implemented by our own Kixunil:
https://github.com/rust-lang/rust/pull/92657
What changed, and why it matters
This commit is a small cleanup that replaces some old-style pointer type conversions with newer, safer Rust standard-library methods. It does not fix a known security bug, but it reduces the chance that a future code change accidentally introduces a dangerous pointer mistake. The actual behavior of the program should be unchanged.
No urgent action required. Treat as normal code-quality/refactoring review. If auditing, verify that the new `cast` calls preserve the same mutability as the original `as` casts and that the added safety comments accurately describe the invariants.
Security signals we found
Replaces permissive `as` pointer casts with stricter `ptr::cast`
Adds or improves SAFETY comments around raw-pointer usage in ArrayVec
No functional change intended; defensive hardening only
No mention of CVE, bug report, exploit, or security fix in commit message
Evidence from the diff
The patch replaces as *const T / as *mut T style raw-pointer casts with ptr::cast::<T>() in two files: an SSE-accelerated SHA-256 implementation and an internal ArrayVec. cast() preserves the original mutability and only changes the pointee type, making it harder to accidentally change *const to *mut or vice versa. The ArrayVec changes also add explicit safety comments and switch from reference-to-pointer transmutes to slice::from_raw_parts/from_raw_parts_mut. The commit is purely defensive/refactoring; no vulnerability is described or fixed.
Changed components
hashes/src/sha256/crypto.rsinternals/src/array_vec.rsInspect captured patch +20 / −12
diff --git a/hashes/src/sha256/crypto.rs b/hashes/src/sha256/crypto.rs
index 8bb0d90c..5f208f06 100644
--- a/hashes/src/sha256/crypto.rs
+++ b/hashes/src/sha256/crypto.rs
@@ -292,8 +292,8 @@ impl HashEngine {
// Load initial values
// CAST SAFETY: loadu_si128 documentation states that mem_addr does not
// need to be aligned on any particular boundary.
- tmp = _mm_loadu_si128(self.h.as_ptr().add(0) as *const __m128i);
- state1 = _mm_loadu_si128(self.h.as_ptr().add(4) as *const __m128i);
+ tmp = _mm_loadu_si128(self.h.as_ptr().add(0).cast::<__m128i>());
+ state1 = _mm_loadu_si128(self.h.as_ptr().add(4).cast::<__m128i>());
tmp = _mm_shuffle_epi32(tmp, 0xB1); // CDAB
state1 = _mm_shuffle_epi32(state1, 0x1B); // EFGH
@@ -307,7 +307,7 @@ impl HashEngine {
cdgh_save = state1;
// Rounds 0-3
- msg = _mm_loadu_si128(self.buffer.as_ptr().add(block_offset) as *const __m128i);
+ msg = _mm_loadu_si128(self.buffer.as_ptr().add(block_offset).cast::<__m128i>());
msg0 = _mm_shuffle_epi8(msg, MASK);
msg = _mm_add_epi32(
msg0,
@@ -318,7 +318,7 @@ impl HashEngine {
state0 = _mm_sha256rnds2_epu32(state0, state1, msg);
// Rounds 4-7
- msg1 = _mm_loadu_si128(self.buffer.as_ptr().add(block_offset + 16) as *const __m128i);
+ msg1 = _mm_loadu_si128(self.buffer.as_ptr().add(block_offset + 16).cast::<__m128i>());
msg1 = _mm_shuffle_epi8(msg1, MASK);
msg = _mm_add_epi32(
msg1,
@@ -330,7 +330,7 @@ impl HashEngine {
msg0 = _mm_sha256msg1_epu32(msg0, msg1);
// Rounds 8-11
- msg2 = _mm_loadu_si128(self.buffer.as_ptr().add(block_offset + 32) as *const __m128i);
+ msg2 = _mm_loadu_si128(self.buffer.as_ptr().add(block_offset + 32).cast::<__m128i>());
msg2 = _mm_shuffle_epi8(msg2, MASK);
msg = _mm_add_epi32(
msg2,
@@ -342,7 +342,7 @@ impl HashEngine {
msg1 = _mm_sha256msg1_epu32(msg1, msg2);
// Rounds 12-15
- msg3 = _mm_loadu_si128(self.buffer.as_ptr().add(block_offset + 48) as *const __m128i);
+ msg3 = _mm_loadu_si128(self.buffer.as_ptr().add(block_offset + 48).cast::<__m128i>());
msg3 = _mm_shuffle_epi8(msg3, MASK);
msg = _mm_add_epi32(
msg3,
@@ -519,8 +519,8 @@ impl HashEngine {
// Save state
// CAST SAFETY: storeu_si128 documentation states that mem_addr does not
// need to be aligned on any particular boundary.
- _mm_storeu_si128(self.h.as_mut_ptr().add(0) as *mut __m128i, state0);
- _mm_storeu_si128(self.h.as_mut_ptr().add(4) as *mut __m128i, state1);
+ _mm_storeu_si128(self.h.as_mut_ptr().add(0).cast::<__m128i>(), state0);
+ _mm_storeu_si128(self.h.as_mut_ptr().add(4).cast::<__m128i>(), state1);
}
// Algorithm copied from libsecp256k1
diff --git a/internals/src/array_vec.rs b/internals/src/array_vec.rs
index c0a036d8..9e402b02 100644
--- a/internals/src/array_vec.rs
+++ b/internals/src/array_vec.rs
@@ -42,15 +42,20 @@ mod safety_boundary {
}
/// Returns a reference to the underlying data.
- #[allow(clippy::incompatible_msrv)] // Clippy doesn't play nicely with `cond_const!`.
pub const fn as_slice(&self) -> &[T] {
- let ptr = &self.data as *const _ as *const T;
+ // transmute needed; see https://github.com/rust-lang/rust/issues/63569
+ // SAFETY: self.len is chosen such that everything is initialized up to len,
+ // and MaybeUninit<T> has the same representation as T.
+ let ptr = self.data.as_ptr().cast::<T>();
unsafe { core::slice::from_raw_parts(ptr, self.len) }
}
/// Returns a mutable reference to the underlying data.
pub fn as_mut_slice(&mut self) -> &mut [T] {
- unsafe { &mut *(&mut self.data[..self.len] as *mut _ as *mut [T]) }
+ // SAFETY: self.len is chosen such that everything is initialized up to len,
+ // and MaybeUninit<T> has the same representation as T.
+ let ptr = self.data.as_mut_ptr().cast::<T>();
+ unsafe { core::slice::from_raw_parts_mut(ptr, self.len) }
}
/// Adds an element into `self`.
@@ -73,7 +78,10 @@ mod safety_boundary {
let new_len = self.len.checked_add(slice.len()).expect("integer/buffer overflow");
assert!(new_len <= CAP, "buffer overflow");
// SAFETY: MaybeUninit<T> has the same layout as T
- let slice = unsafe { &*(slice as *const _ as *const [MaybeUninit<T>]) };
+ let slice = unsafe {
+ let ptr = slice.as_ptr();
+ core::slice::from_raw_parts(ptr.cast::<MaybeUninit<T>>(), slice.len())
+ };
self.data[self.len..new_len].copy_from_slice(slice);
self.len = new_len;
}
Why this scored 18/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.