From 83c713bff022acabc4dab30510f35122e29d0f53 Mon Sep 17 00:00:00 2001
From: ShE3py <52315535+she3py@users.noreply.github.com>
Date: Fri, 18 Aug 2023 16:04:53 +0200
Subject: [PATCH] Fix UB in `std::sys::os::getenv()`
---
library/std/src/sys/solid/os.rs | 30 +++++++++++++++++++-----------
library/std/src/sys/unix/os.rs | 21 +++++++++++++--------
library/std/src/sys/wasi/os.rs | 23 +++++++++++++++--------
3 files changed, 47 insertions(+), 27 deletions(-)
diff --git a/library/std/src/sys/solid/os.rs b/library/std/src/sys/solid/os.rs
index 717c08434a89f..9f4e66d628b4b 100644
--- a/library/std/src/sys/solid/os.rs
+++ b/library/std/src/sys/solid/os.rs
@@ -81,6 +81,10 @@ pub fn current_exe() -> io::Result<PathBuf> {
static ENV_LOCK: RwLock<()> = RwLock::new(());
+pub fn env_read_lock() -> impl Drop {
+ ENV_LOCK.read().unwrap_or_else(PoisonError::into_inner)
+}
+
pub struct Env {
iter: vec::IntoIter<(OsString, OsString)>,
}
@@ -134,7 +138,7 @@ pub fn env() -> Env {
}
unsafe {
- let _guard = ENV_LOCK.read();
+ let _guard = env_read_lock();
let mut result = Vec::new();
if !environ.is_null() {
while !(*environ).is_null() {
@@ -168,17 +172,21 @@ pub fn env() -> Env {
pub fn getenv(k: &OsStr) -> Option<OsString> {
// environment variables with a nul byte can't be set, so their value is
// always None as well
- let s = run_with_cstr(k.as_bytes(), |k| {
- let _guard = ENV_LOCK.read();
- Ok(unsafe { libc::getenv(k.as_ptr()) } as *const libc::c_char)
- })
- .ok()?;
+ run_with_cstr(k.as_bytes(), |k| {
+ let _guard = env_read_lock();
+ let v = unsafe { libc::getenv(k.as_ptr()) } as *const libc::c_char;
- if s.is_null() {
- None
- } else {
- Some(OsStringExt::from_vec(unsafe { CStr::from_ptr(s) }.to_bytes().to_vec()))
- }
+ if v.is_null() {
+ Ok(None)
+ } else {
+ // SAFETY: `v` cannot be mutated while executing this line since we've a read lock
+ let bytes = unsafe { CStr::from_ptr(v) }.to_bytes().to_vec();
+
+ Ok(Some(OsStringExt::from_vec(bytes)))
+ }
+ })
+ .ok()
+ .flatten()
}
pub fn setenv(k: &OsStr, v: &OsStr) -> io::Result<()> {
diff --git a/library/std/src/sys/unix/os.rs b/library/std/src/sys/unix/os.rs
index 215f63d04f77b..57e1a36dace9b 100644
--- a/library/std/src/sys/unix/os.rs
+++ b/library/std/src/sys/unix/os.rs
@@ -594,16 +594,21 @@ pub fn env() -> Env {
pub fn getenv(k: &OsStr) -> Option<OsString> {
// environment variables with a nul byte can't be set, so their value is
// always None as well
- let s = run_with_cstr(k.as_bytes(), |k| {
+ run_with_cstr(k.as_bytes(), |k| {
let _guard = env_read_lock();
- Ok(unsafe { libc::getenv(k.as_ptr()) } as *const libc::c_char)
+ let v = unsafe { libc::getenv(k.as_ptr()) } as *const libc::c_char;
+
+ if v.is_null() {
+ Ok(None)
+ } else {
+ // SAFETY: `v` cannot be mutated while executing this line since we've a read lock
+ let bytes = unsafe { CStr::from_ptr(v) }.to_bytes().to_vec();
+
+ Ok(Some(OsStringExt::from_vec(bytes)))
+ }
})
- .ok()?;
- if s.is_null() {
- None
- } else {
- Some(OsStringExt::from_vec(unsafe { CStr::from_ptr(s) }.to_bytes().to_vec()))
- }
+ .ok()
+ .flatten()
}
pub fn setenv(k: &OsStr, v: &OsStr) -> io::Result<()> {
diff --git a/library/std/src/sys/wasi/os.rs b/library/std/src/sys/wasi/os.rs
index e0de284c5e24e..d53bddd8e9dfe 100644
--- a/library/std/src/sys/wasi/os.rs
+++ b/library/std/src/sys/wasi/os.rs
@@ -225,16 +225,23 @@ pub fn env() -> Env {
}
pub fn getenv(k: &OsStr) -> Option<OsString> {
- let s = run_with_cstr(k.as_bytes(), |k| unsafe {
+ // environment variables with a nul byte can't be set, so their value is
+ // always None as well
+ run_with_cstr(k.as_bytes(), |k| {
let _guard = env_read_lock();
- Ok(libc::getenv(k.as_ptr()) as *const libc::c_char)
+ let v = unsafe { libc::getenv(k.as_ptr()) } as *const libc::c_char;
+
+ if v.is_null() {
+ Ok(None)
+ } else {
+ // SAFETY: `v` cannot be mutated while executing this line since we've a read lock
+ let bytes = unsafe { CStr::from_ptr(v) }.to_bytes().to_vec();
+
+ Ok(Some(OsStringExt::from_vec(bytes)))
+ }
})
- .ok()?;
- if s.is_null() {
- None
- } else {
- Some(OsStringExt::from_vec(unsafe { CStr::from_ptr(s) }.to_bytes().to_vec()))
- }
+ .ok()
+ .flatten()
}
pub fn setenv(k: &OsStr, v: &OsStr) -> io::Result<()> {