From c2aac4c5d56fe14700a6f48ae9a444104e5d26b5 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Mon, 7 Nov 2022 17:56:56 -0800 Subject: [PATCH] Fix `realpath` and `strerror_r` to handle uninitialized buffers. (#143) * Fix `realpath` and `strerror_r` to handle uninitialized buffers. Use `copy_nonoverlapping` instead of `copy_from_slice` in a few places to avoid creating Rust slices from buffers which may be uninitialized, which has UB. * Convert read-style functions to use copy_nonoverlapping. --- c-scape/src/error_str.rs | 162 ++++++++++++++++++------------------- c-scape/src/fs/readlink.rs | 3 +- c-scape/src/fs/realpath.rs | 4 +- c-scape/src/io/read.rs | 27 +++++-- c-scape/src/lib.rs | 30 ++++--- c-scape/src/net/mod.rs | 23 +++++- 6 files changed, 143 insertions(+), 106 deletions(-) diff --git a/c-scape/src/error_str.rs b/c-scape/src/error_str.rs index 40a884ea..e50612e5 100644 --- a/c-scape/src/error_str.rs +++ b/c-scape/src/error_str.rs @@ -7,91 +7,91 @@ pub(crate) const fn error_str(e: Errno) -> Option<&'static str> { // Recognize errors documented in POSIX and use the documented strings. // Some(match e { - Errno::TOOBIG => "Argument list too long.", - Errno::ACCESS => "Permission denied.", - Errno::ADDRINUSE => "Address in use.", - Errno::ADDRNOTAVAIL => "Address not available.", - Errno::AFNOSUPPORT => "Address family not supported.", - Errno::AGAIN => "Resource unavailable, try again.", - Errno::ALREADY => "Connection already in progress.", - Errno::BADF => "Bad file descriptor.", - Errno::BADMSG => "Bad message.", - Errno::BUSY => "Device or resource busy.", - Errno::CANCELED => "Operation canceled.", - Errno::CHILD => "No child processes.", - Errno::CONNABORTED => "Connection aborted.", - Errno::CONNREFUSED => "Connection refused.", - Errno::CONNRESET => "Connection reset.", - Errno::DEADLK => "Resource deadlock would occur.", - Errno::DESTADDRREQ => "Destination address required.", - Errno::DOM => "Mathematics argument out of domain of function.", - Errno::DQUOT => "Reserved.", - Errno::EXIST => "File exists.", - Errno::FAULT => "Bad address.", - Errno::FBIG => "File too large.", - Errno::HOSTUNREACH => "Host is unreachable.", - Errno::IDRM => "Identifier removed.", - Errno::ILSEQ => "Illegal byte sequence.", - Errno::INPROGRESS => "Operation in progress.", - Errno::INTR => "Interrupted function.", - Errno::INVAL => "Invalid argument.", - Errno::IO => "I/O error.", - Errno::ISCONN => "Socket is connected.", - Errno::ISDIR => "Is a directory.", - Errno::LOOP => "Too many levels of symbolic links.", - Errno::MFILE => "File descriptor value too large.", - Errno::MLINK => "Too many links.", - Errno::MSGSIZE => "Message too large.", - Errno::MULTIHOP => "Reserved.", - Errno::NAMETOOLONG => "Filename too long.", - Errno::NETDOWN => "Network is down.", - Errno::NETRESET => "Connection aborted by network.", - Errno::NETUNREACH => "Network unreachable.", - Errno::NFILE => "Too many files open in system.", - Errno::NOBUFS => "No buffer space available.", + Errno::TOOBIG => "Argument list too long", + Errno::ACCESS => "Permission denied", + Errno::ADDRINUSE => "Address in use", + Errno::ADDRNOTAVAIL => "Address not available", + Errno::AFNOSUPPORT => "Address family not supported", + Errno::AGAIN => "Resource unavailable, try again", + Errno::ALREADY => "Connection already in progress", + Errno::BADF => "Bad file descriptor", + Errno::BADMSG => "Bad message", + Errno::BUSY => "Device or resource busy", + Errno::CANCELED => "Operation canceled", + Errno::CHILD => "No child processes", + Errno::CONNABORTED => "Connection aborted", + Errno::CONNREFUSED => "Connection refused", + Errno::CONNRESET => "Connection reset", + Errno::DEADLK => "Resource deadlock would occur", + Errno::DESTADDRREQ => "Destination address required", + Errno::DOM => "Mathematics argument out of domain of function", + Errno::DQUOT => "Reserved", + Errno::EXIST => "File exists", + Errno::FAULT => "Bad address", + Errno::FBIG => "File too large", + Errno::HOSTUNREACH => "Host is unreachable", + Errno::IDRM => "Identifier removed", + Errno::ILSEQ => "Illegal byte sequence", + Errno::INPROGRESS => "Operation in progress", + Errno::INTR => "Interrupted function", + Errno::INVAL => "Invalid argument", + Errno::IO => "I/O error", + Errno::ISCONN => "Socket is connected", + Errno::ISDIR => "Is a directory", + Errno::LOOP => "Too many levels of symbolic links", + Errno::MFILE => "File descriptor value too large", + Errno::MLINK => "Too many links", + Errno::MSGSIZE => "Message too large", + Errno::MULTIHOP => "Reserved", + Errno::NAMETOOLONG => "Filename too long", + Errno::NETDOWN => "Network is down", + Errno::NETRESET => "Connection aborted by network", + Errno::NETUNREACH => "Network unreachable", + Errno::NFILE => "Too many files open in system", + Errno::NOBUFS => "No buffer space available", #[cfg(not(target_os = "wasi"))] - Errno::NODATA => "No message is available on the STREAM head read queue.", - Errno::NODEV => "No such device.", - Errno::NOENT => "No such file or directory.", - Errno::NOEXEC => "Executable file format error.", - Errno::NOLCK => "No locks available.", - Errno::NOLINK => "Reserved.", - Errno::NOMEM => "Not enough space.", - Errno::NOMSG => "No message of the desired type.", - Errno::NOPROTOOPT => "Protocol not available.", - Errno::NOSPC => "No space left on device.", + Errno::NODATA => "No message is available on the STREAM head read queue", + Errno::NODEV => "No such device", + Errno::NOENT => "No such file or directory", + Errno::NOEXEC => "Executable file format error", + Errno::NOLCK => "No locks available", + Errno::NOLINK => "Reserved", + Errno::NOMEM => "Not enough space", + Errno::NOMSG => "No message of the desired type", + Errno::NOPROTOOPT => "Protocol not available", + Errno::NOSPC => "No space left on device", #[cfg(not(target_os = "wasi"))] - Errno::NOSR => "No STREAM resources.", + Errno::NOSR => "No STREAM resources", #[cfg(not(target_os = "wasi"))] - Errno::NOSTR => "Not a STREAM.", - Errno::NOSYS => "Functionality not supported.", - Errno::NOTCONN => "The socket is not connected.", - Errno::NOTDIR => "Not a directory or a symbolic link to a directory.", - Errno::NOTEMPTY => "Directory not empty.", - Errno::NOTRECOVERABLE => "State not recoverable.", - Errno::NOTSOCK => "Not a socket.", - Errno::NOTSUP => "Not supported.", - Errno::NOTTY => "Inappropriate I/O control operation.", - Errno::NXIO => "No such device or address.", - //Errno::OPNOTSUPP => "Operation not supported on socket.", // same as `NOTSUP` - Errno::OVERFLOW => "Value too large to be stored in data type.", - Errno::OWNERDEAD => "Previous owner died.", - Errno::PERM => "Operation not permitted.", - Errno::PIPE => "Broken pipe.", - Errno::PROTO => "Protocol error.", - Errno::PROTONOSUPPORT => "Protocol not supported.", - Errno::PROTOTYPE => "Protocol wrong type for socket.", - Errno::RANGE => "Result too large.", - Errno::ROFS => "Read-only file system.", - Errno::SPIPE => "Invalid seek.", - Errno::SRCH => "No such process.", - Errno::STALE => "Reserved.", + Errno::NOSTR => "Not a STREAM", + Errno::NOSYS => "Functionality not supported", + Errno::NOTCONN => "The socket is not connected", + Errno::NOTDIR => "Not a directory or a symbolic link to a directory", + Errno::NOTEMPTY => "Directory not empty", + Errno::NOTRECOVERABLE => "State not recoverable", + Errno::NOTSOCK => "Not a socket", + Errno::NOTSUP => "Not supported", + Errno::NOTTY => "Inappropriate I/O control operation", + Errno::NXIO => "No such device or address", + //Errno::OPNOTSUPP => "Operation not supported on socket", // same as `NOTSUP` + Errno::OVERFLOW => "Value too large to be stored in data type", + Errno::OWNERDEAD => "Previous owner died", + Errno::PERM => "Operation not permitted", + Errno::PIPE => "Broken pipe", + Errno::PROTO => "Protocol error", + Errno::PROTONOSUPPORT => "Protocol not supported", + Errno::PROTOTYPE => "Protocol wrong type for socket", + Errno::RANGE => "Result too large", + Errno::ROFS => "Read-only file system", + Errno::SPIPE => "Invalid seek", + Errno::SRCH => "No such process", + Errno::STALE => "Reserved", #[cfg(not(target_os = "wasi"))] - Errno::TIME => "Stream ioctl() timeout.", - Errno::TIMEDOUT => "Connection timed out.", - Errno::TXTBSY => "Text file busy.", - //Errno::WOULDBLOCK => "Operation would block.", // same as `AGAIN` - Errno::XDEV => "Cross-device link.", + Errno::TIME => "Stream ioctl() timeout", + Errno::TIMEDOUT => "Connection timed out", + Errno::TXTBSY => "Text file busy", + //Errno::WOULDBLOCK => "Operation would block", // same as `AGAIN` + Errno::XDEV => "Cross-device link", #[cfg(target_os = "wasi")] Errno::NOTCAPABLE => "Capabilities insufficient", _ => return None, diff --git a/c-scape/src/fs/readlink.rs b/c-scape/src/fs/readlink.rs index edeb42ad..893525a1 100644 --- a/c-scape/src/fs/readlink.rs +++ b/c-scape/src/fs/readlink.rs @@ -1,5 +1,6 @@ use alloc::vec::Vec; use core::ffi::CStr; +use core::ptr::copy_nonoverlapping; use rustix::fd::BorrowedFd; use libc::{c_char, c_int}; @@ -32,6 +33,6 @@ unsafe extern "C" fn readlinkat( }; let bytes = path.as_bytes(); let min = core::cmp::min(bytes.len(), bufsiz); - core::slice::from_raw_parts_mut(buf.cast(), min).copy_from_slice(bytes); + copy_nonoverlapping(bytes.as_ptr(), buf.cast(), min); min as isize } diff --git a/c-scape/src/fs/realpath.rs b/c-scape/src/fs/realpath.rs index 8e621591..a7d45cb9 100644 --- a/c-scape/src/fs/realpath.rs +++ b/c-scape/src/fs/realpath.rs @@ -1,6 +1,6 @@ use core::ffi::CStr; -use core::ptr::null_mut; +use core::ptr::{copy_nonoverlapping, null_mut}; use errno::{set_errno, Errno}; use libc::{c_char, c_void, malloc, memcpy}; @@ -21,7 +21,7 @@ unsafe extern "C" fn realpath(path: *const c_char, resolved_path: *mut c_char) - set_errno(Errno(libc::ENOMEM)); return null_mut(); } - core::slice::from_raw_parts_mut(ptr, len).copy_from_slice(&buf[..len]); + copy_nonoverlapping(buf.as_ptr().cast(), ptr, len); *ptr.add(len) = b'\0'; ptr.cast::() } else { diff --git a/c-scape/src/io/read.rs b/c-scape/src/io/read.rs index f8f3bae2..0507ec6c 100644 --- a/c-scape/src/io/read.rs +++ b/c-scape/src/io/read.rs @@ -1,6 +1,8 @@ use rustix::fd::BorrowedFd; use rustix::io::IoSliceMut; +use alloc::vec; +use core::ptr::copy_nonoverlapping; use core::slice; use errno::{set_errno, Errno}; use libc::{c_int, c_void, iovec, off64_t, off_t}; @@ -11,11 +13,15 @@ use crate::convert_res; unsafe extern "C" fn read(fd: c_int, ptr: *mut c_void, len: usize) -> isize { libc!(libc::read(fd, ptr, len)); - match convert_res(rustix::io::read( - BorrowedFd::borrow_raw(fd), - slice::from_raw_parts_mut(ptr.cast::(), len), - )) { - Some(nread) => nread as isize, + // `slice::from_raw_parts_mut` assumes that the memory is initialized, + // which our C API here doesn't guarantee. Since rustix currently requires + // a slice, use a temporary copy. + let mut tmp = vec![0u8; len]; + match convert_res(rustix::io::read(BorrowedFd::borrow_raw(fd), &mut tmp)) { + Some(nread) => { + copy_nonoverlapping(tmp.as_ptr(), ptr.cast::(), len); + nread as isize + } None => -1, } } @@ -53,12 +59,19 @@ unsafe extern "C" fn pread(fd: c_int, ptr: *mut c_void, len: usize, offset: off_ unsafe extern "C" fn pread64(fd: c_int, ptr: *mut c_void, len: usize, offset: off64_t) -> isize { libc!(libc::pread64(fd, ptr, len, offset)); + // `slice::from_raw_parts_mut` assumes that the memory is initialized, + // which our C API here doesn't guarantee. Since rustix currently requires + // a slice, use a temporary copy. + let mut tmp = vec![0u8; len]; match convert_res(rustix::io::pread( BorrowedFd::borrow_raw(fd), - slice::from_raw_parts_mut(ptr.cast::(), len), + &mut tmp, offset as u64, )) { - Some(nread) => nread as isize, + Some(nread) => { + copy_nonoverlapping(tmp.as_ptr(), ptr.cast::(), len); + nread as isize + } None => -1, } } diff --git a/c-scape/src/lib.rs b/c-scape/src/lib.rs index e3dfecfa..6c641859 100644 --- a/c-scape/src/lib.rs +++ b/c-scape/src/lib.rs @@ -49,8 +49,7 @@ use core::convert::TryInto; use core::ffi::c_void; #[cfg(not(target_os = "wasi"))] use core::mem::{size_of, zeroed}; -use core::ptr::{self, null, null_mut}; -use core::slice; +use core::ptr::{self, copy_nonoverlapping, null, null_mut}; use errno::{set_errno, Errno}; use error_str::error_str; #[cfg(target_vendor = "mustang")] @@ -133,14 +132,18 @@ unsafe extern "C" fn strerror(errnum: c_int) -> *mut c_char { unsafe extern "C" fn __xpg_strerror_r(errnum: c_int, buf: *mut c_char, buflen: usize) -> c_int { libc!(libc::strerror_r(errnum, buf, buflen)); + if buflen == 0 { + return libc::ERANGE; + } + let message = match error_str(rustix::io::Errno::from_raw_os_error(errnum)) { Some(s) => s.to_owned(), None => format!("Unknown error {}", errnum), }; - let min = core::cmp::min(buflen, message.len()); - let out = slice::from_raw_parts_mut(buf.cast::(), min); - out.copy_from_slice(message.as_bytes()); - out[out.len() - 1] = b'\0'; + + let min = core::cmp::min(buflen - 1, message.len()); + copy_nonoverlapping(message.as_ptr().cast(), buf, min); + buf.add(min).write(b'\0' as libc::c_char); 0 } @@ -156,11 +159,16 @@ unsafe extern "C" fn getrandom(buf: *mut c_void, buflen: usize, flags: u32) -> i } let flags = rustix::rand::GetRandomFlags::from_bits(flags & !0x4).unwrap(); - match convert_res(rustix::rand::getrandom( - slice::from_raw_parts_mut(buf.cast::(), buflen), - flags, - )) { - Some(num) => num as isize, + + // `slice::from_raw_parts_mut` assumes that the memory is initialized, + // which our C API here doesn't guarantee. Since rustix currently requires + // a slice, use a temporary copy. + let mut tmp = alloc::vec![0u8; buflen]; + match convert_res(rustix::rand::getrandom(&mut tmp, flags)) { + Some(num) => { + core::ptr::copy_nonoverlapping(tmp.as_ptr(), buf.cast::(), buflen); + num as isize + } None => -1, } } diff --git a/c-scape/src/net/mod.rs b/c-scape/src/net/mod.rs index 163ea74c..391295f7 100644 --- a/c-scape/src/net/mod.rs +++ b/c-scape/src/net/mod.rs @@ -2,11 +2,12 @@ mod inet; #[cfg(feature = "sync-resolve")] use alloc::string::ToString; +use alloc::vec; use core::convert::TryInto; use core::ffi::{c_void, CStr}; #[cfg(not(target_os = "wasi"))] use core::mem::{size_of, zeroed}; -use core::ptr::null_mut; +use core::ptr::{copy_nonoverlapping, null_mut}; use core::slice; use errno::{set_errno, Errno}; use libc::{c_char, c_int, c_uint}; @@ -590,12 +591,20 @@ unsafe extern "C" fn recv(fd: c_int, ptr: *mut c_void, len: usize, flags: c_int) libc!(libc::recv(fd, ptr, len, flags)); let flags = RecvFlags::from_bits(flags as _).unwrap(); + + // `slice::from_raw_parts_mut` assumes that the memory is initialized, + // which our C API here doesn't guarantee. Since rustix currently requires + // a slice, use a temporary copy. + let mut tmp = vec![0u8; len]; match convert_res(rustix::net::recv( BorrowedFd::borrow_raw(fd), - slice::from_raw_parts_mut(ptr.cast::(), len), + &mut tmp, flags, )) { - Some(nread) => nread as isize, + Some(nread) => { + copy_nonoverlapping(tmp.as_ptr(), ptr.cast::(), len); + nread as isize + } None => -1, } } @@ -614,12 +623,18 @@ unsafe extern "C" fn recvfrom( libc!(libc::recvfrom(fd, buf, len, flags, from.cast(), from_len)); let flags = RecvFlags::from_bits(flags as _).unwrap(); + + // `slice::from_raw_parts_mut` assumes that the memory is initialized, + // which our C API here doesn't guarantee. Since rustix currently requires + // a slice, use a temporary copy. + let mut tmp = vec![0u8; len]; match convert_res(rustix::net::recvfrom( BorrowedFd::borrow_raw(fd), - slice::from_raw_parts_mut(buf.cast::(), len), + &mut tmp, flags, )) { Some((nread, addr)) => { + copy_nonoverlapping(tmp.as_ptr(), buf.cast::(), len); if let Some(addr) = addr { let encoded_len = addr.write(from); *from_len = encoded_len.try_into().unwrap();