mirror of
https://github.com/rust-lang/rust.git
synced 2026-04-27 18:57:42 +03:00
Rollup merge of #155707 - Manishearth:cstring-vuln, r=Mark-Simulacrum
Fix minor panic-unsoundness in CString::clone_into `CString` must always contain a null byte, calling `mem::take` on its inner allocation puts it in an invalid state (causing UB if e.g. it hits `CString::drop`) that can be observed if the allocator panics. Unfortunately, this solution allocates an intermediate 1-element `Box`. I'm not sure of a clean way to avoid that additional allocation; we could directly `realloc` if we want but it's tricky. Might be something we can do with `ManuallyDrop`. I do have a gnarly miri test for this that uses a panicky allocator, but I'm not sure where it would go. Happy to push it up if someone has a suggestion. Bug discovered by Rust Foundation Security using AI. I'm just helping with the patch as a member of wg-security-response. We do not believe this bug needs embargo, it is a soundness fix for hard-to-trigger unsoundness.
This commit is contained in:
@@ -1077,9 +1077,17 @@ fn to_owned(&self) -> CString {
|
||||
}
|
||||
|
||||
fn clone_into(&self, target: &mut CString) {
|
||||
let mut b = mem::take(&mut target.inner).into_vec();
|
||||
self.to_bytes_with_nul().clone_into(&mut b);
|
||||
target.inner = b.into_boxed_slice();
|
||||
let src = self.to_bytes_with_nul();
|
||||
// If the lengths match, we can reuse the existing allocation without any overhead.
|
||||
if target.inner.len() == src.len() {
|
||||
target.inner.copy_from_slice(src);
|
||||
} else {
|
||||
// Reuse the existing allocation's capacity by converting to a Vec.
|
||||
// We temporarily replace `target` with a valid dummy to remain panic-safe.
|
||||
let mut b = mem::replace(&mut target.inner, Box::new([0])).into_vec();
|
||||
self.to_bytes_with_nul().clone_into(&mut b);
|
||||
target.inner = b.into_boxed_slice();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user