Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/podstring_utf8_soundness.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
pina_pod_primitives: fix
---

Remove the unsound `Deref<Target = str>` and `AsRef<str>` impls from `PodString`. Because `PodString` is `bytemuck::Pod`, bytes loaded from untrusted account data may not be valid UTF-8, and the removed impls produced a `&str` via `from_utf8_unchecked` — undefined behavior. Use `try_as_str()` for validated access, or `as_str_unchecked()` (unsafe) for unchecked access.
25 changes: 8 additions & 17 deletions crates/pina_pod_primitives/src/string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,11 +103,16 @@ impl<const N: usize, const PFX: usize> PodString<N, PFX> {
N
}

/// Returns the string as a `&str`.
/// Returns the string as a `&str` without validating UTF-8.
///
/// This is the only way to obtain a `&str` view without validation —
/// `PodString` deliberately does not implement `Deref<Target = str>` or
/// `AsRef<str>`, because the stored bytes may be arbitrary when the value
/// is loaded from untrusted account data via `Pod`.
///
/// # Safety
/// This assumes the stored bytes are valid UTF-8. For untrusted account
/// data, use `try_as_str()` instead.
/// The stored bytes must be valid UTF-8. For untrusted account data, use
/// `try_as_str()` instead.
#[inline]
pub unsafe fn as_str_unchecked(&self) -> &str {
unsafe {
Expand Down Expand Up @@ -197,20 +202,6 @@ impl<const N: usize, const PFX: usize> Default for PodString<N, PFX> {
}
}

impl<const N: usize, const PFX: usize> core::ops::Deref for PodString<N, PFX> {
type Target = str;

fn deref(&self) -> &str {
unsafe { self.as_str_unchecked() }
}
}

impl<const N: usize, const PFX: usize> AsRef<str> for PodString<N, PFX> {
fn as_ref(&self) -> &str {
unsafe { self.as_str_unchecked() }
}
}

impl<const N: usize, const PFX: usize> AsRef<[u8]> for PodString<N, PFX> {
fn as_ref(&self) -> &[u8] {
self.as_bytes()
Expand Down
76 changes: 76 additions & 0 deletions crates/pina_pod_primitives/src/tests/string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,3 +56,79 @@ fn pod_string_bytemuck_roundtrip() {
let restored = unsafe { &*(bytes.as_ptr() as *const PodString<32>) };
assert_eq!(restored.try_as_str().unwrap(), "test");
}

// ---------------------------------------------------------------------------
// UTF-8 soundness: PodString loaded from untrusted bytes
// ---------------------------------------------------------------------------

/// A `PodString` loaded from arbitrary account data may contain invalid
/// UTF-8. `try_as_str()` must reject it rather than producing a `&str`.
#[test]
fn pod_string_invalid_utf8_rejected_by_try_as_str() {
// Length prefix 2, data bytes [0xff, 0xfe] — invalid UTF-8.
let bytes = [2u8, 0xff, 0xfe];
let pod = try_from_bytes::<PodString<2>>(&bytes)
.unwrap_or_else(|e| panic!("try_from_bytes failed for {bytes:?}: {e}"));
assert_eq!(pod.len(), 2);
assert!(matches!(
pod.try_as_str(),
Err(PodCollectionError::InvalidUtf8)
));
}

/// A truncated multi-byte sequence is also invalid UTF-8.
#[test]
fn pod_string_incomplete_utf8_rejected_by_try_as_str() {
// Length prefix 1, data byte 0xc3 — a lead byte with no continuation.
let bytes = [1u8, 0xc3];
let pod = try_from_bytes::<PodString<1>>(&bytes)
.unwrap_or_else(|e| panic!("try_from_bytes failed for {bytes:?}: {e}"));
assert!(matches!(
pod.try_as_str(),
Err(PodCollectionError::InvalidUtf8)
));
}

/// Safe traits must never panic or produce a `&str` from invalid UTF-8.
#[test]
fn pod_string_invalid_utf8_safe_traits_do_not_panic() {
let bytes = [2u8, 0xff, 0xfe];
let pod = try_from_bytes::<PodString<2>>(&bytes)
.unwrap_or_else(|e| panic!("try_from_bytes failed for {bytes:?}: {e}"));

// Debug and Display fall back to a placeholder.
assert_eq!(std::format!("{pod:?}"), "PodString { len: 2 }");
assert_eq!(std::format!("{pod}"), "<invalid utf8>");

// Byte access and byte-based comparison remain total and safe.
assert_eq!(pod.as_bytes(), &[0xff, 0xfe]);
assert_eq!(<PodString<2> as AsRef<[u8]>>::as_ref(&pod), &[0xff, 0xfe]);
assert_eq!(pod, pod);
assert_ne!(pod, "valid");
}

/// Valid multi-byte UTF-8 survives a Pod round-trip.
#[test]
fn pod_string_valid_utf8_roundtrip_via_pod() {
let mut s = PodString::<32>::default();
s.set("héllo wörld");
let bytes = bytemuck::bytes_of(&s);
let restored = try_from_bytes::<PodString<32>>(bytes)
.unwrap_or_else(|e| panic!("try_from_bytes failed: {e}"));
assert_eq!(
restored
.try_as_str()
.unwrap_or_else(|e| panic!("invalid UTF-8: {e}")),
"héllo wörld"
);
}

/// `as_str_unchecked` remains available as an explicit unsafe escape hatch.
#[test]
fn pod_string_as_str_unchecked_valid() {
let mut s = PodString::<32>::default();
s.set("hello");
// SAFETY: "hello" is valid UTF-8.
let s = unsafe { s.as_str_unchecked() };
assert_eq!(s, "hello");
}
Loading