diff --git a/src/libos/src/entry.rs b/src/libos/src/entry.rs index a80ad6e5..bce468c9 100644 --- a/src/libos/src/entry.rs +++ b/src/libos/src/entry.rs @@ -135,7 +135,9 @@ fn parse_log_level(level_chars: *const c_char) -> Result { } let level_string = { - let level_cstring = clone_cstring_safely(level_chars)?; + // level_chars has been guaranteed to be inside enclave + // and null terminated by ECall + let level_cstring = CString::from(unsafe { CStr::from_ptr(level_chars) }); level_cstring .into_string() .map_err(|e| errno!(EINVAL, "log_level contains valid utf-8 data"))? @@ -158,7 +160,13 @@ fn parse_arguments( host_stdio_fds: *const HostStdioFds, ) -> Result<(PathBuf, Vec, HostStdioFds)> { let path_buf = { - let path_cstring = clone_cstring_safely(path_ptr)?; + if path_ptr.is_null() { + return_errno!(EINVAL, "empty path"); + } + // path_ptr has been guaranteed to be inside enclave + // and null terminated by ECall + let path_cstring = CString::from(unsafe { CStr::from_ptr(path_ptr) }); + let path_string = path_cstring .into_string() .map_err(|e| errno!(EINVAL, "path contains valid utf-8 data"))?; diff --git a/src/libos/src/fs/syscalls.rs b/src/libos/src/fs/syscalls.rs index fe2c4228..b1f3962f 100644 --- a/src/libos/src/fs/syscalls.rs +++ b/src/libos/src/fs/syscalls.rs @@ -82,7 +82,7 @@ pub fn do_writev(fd: FileDesc, iov: *const iovec_t, count: i32) -> Result count as usize }; - from_user::check_array(iov, count); + from_user::check_array(iov, count)?; let bufs_vec = { let mut bufs_vec = Vec::with_capacity(count); for iov_i in 0..count { @@ -107,7 +107,7 @@ pub fn do_readv(fd: FileDesc, iov: *mut iovec_t, count: i32) -> Result { count as usize }; - from_user::check_array(iov, count); + from_user::check_array(iov, count)?; let mut bufs_vec = { let mut bufs_vec = Vec::with_capacity(count); for iov_i in 0..count { diff --git a/src/libos/src/net/syscalls.rs b/src/libos/src/net/syscalls.rs index 4d956fef..b5dc8e13 100644 --- a/src/libos/src/net/syscalls.rs +++ b/src/libos/src/net/syscalls.rs @@ -170,7 +170,11 @@ pub fn do_select( } pub fn do_poll(fds: *mut libc::pollfd, nfds: libc::nfds_t, timeout: c_int) -> Result { - from_user::check_mut_array(fds, nfds as usize)?; + // It behaves like sleep when fds is null and nfds is zero. + if !fds.is_null() || nfds != 0 { + from_user::check_mut_array(fds, nfds as usize)?; + } + let polls = unsafe { std::slice::from_raw_parts_mut(fds, nfds as usize) }; let n = io_multiplexing::do_poll(polls, timeout)?; diff --git a/src/libos/src/syscall/mod.rs b/src/libos/src/syscall/mod.rs index d556878e..ade0347a 100644 --- a/src/libos/src/syscall/mod.rs +++ b/src/libos/src/syscall/mod.rs @@ -753,13 +753,24 @@ fn do_connect(fd: c_int, addr: *const libc::sockaddr, addr_len: libc::socklen_t) "connect: fd: {}, addr: {:?}, addr_len: {}", fd, addr, addr_len ); + // For SOCK_DGRAM sockets not initiated in connection-mode, + // if address is a null address for the protocol, + // the socket's peer address shall be reset. + let need_check: bool = !addr.is_null(); + if need_check { + check_array(addr as *const u8, addr_len as usize)?; + } + let file_ref = current!().file(fd as FileDesc)?; if let Ok(socket) = file_ref.as_socket() { + if need_check { + check_ptr(addr as *const libc::sockaddr_in)?; + } let ret = try_libc!(libc::ocall::connect(socket.fd(), addr, addr_len)); Ok(ret as isize) } else if let Ok(unix_socket) = file_ref.as_unix_socket() { let addr = addr as *const libc::sockaddr_un; - check_ptr(addr)?; // TODO: check addr_len + check_ptr(addr)?; let path = clone_cstring_safely(unsafe { (&*addr).sun_path.as_ptr() })? .to_string_lossy() .into_owned(); @@ -788,9 +799,21 @@ fn do_accept4( "accept4: fd: {}, addr: {:?}, addr_len: {:?}, flags: {:#x}", fd, addr, addr_len, flags ); + + let need_check: bool = !addr.is_null(); + + if addr.is_null() ^ addr_len.is_null() { + return_errno!(EINVAL, "addr and ddr_len should be both null"); + } + if need_check { + check_mut_array(addr as *mut u8, unsafe { *addr_len } as usize)?; + } + let file_ref = current!().file(fd as FileDesc)?; if let Ok(socket) = file_ref.as_socket() { - let socket = file_ref.as_socket()?; + if need_check { + check_mut_ptr(addr as *mut libc::sockaddr_in)?; + } let new_socket = socket.accept(addr, addr_len, flags)?; let new_file_ref: Arc> = Arc::new(Box::new(new_socket)); @@ -799,8 +822,10 @@ fn do_accept4( Ok(new_fd as isize) } else if let Ok(unix_socket) = file_ref.as_unix_socket() { let addr = addr as *mut libc::sockaddr_un; - check_mut_ptr(addr)?; // TODO: check addr_len - + if need_check { + check_mut_ptr(addr)?; + } + // TODO: handle addr let new_socket = unix_socket.accept()?; let new_file_ref: Arc> = Arc::new(Box::new(new_socket)); let new_fd = current!().add_file(new_file_ref, false); @@ -824,14 +849,19 @@ fn do_shutdown(fd: c_int, how: c_int) -> Result { fn do_bind(fd: c_int, addr: *const libc::sockaddr, addr_len: libc::socklen_t) -> Result { debug!("bind: fd: {}, addr: {:?}, addr_len: {}", fd, addr, addr_len); + if addr.is_null() && addr_len == 0 { + return_errno!(EINVAL, "no address is specified"); + } + check_array(addr as *const u8, addr_len as usize)?; + let file_ref = current!().file(fd as FileDesc)?; if let Ok(socket) = file_ref.as_socket() { - check_ptr(addr)?; // TODO: check addr_len + check_ptr(addr as *const libc::sockaddr_in)?; let ret = try_libc!(libc::ocall::bind(socket.fd(), addr, addr_len)); Ok(ret as isize) } else if let Ok(unix_socket) = file_ref.as_unix_socket() { let addr = addr as *const libc::sockaddr_un; - check_ptr(addr)?; // TODO: check addr_len + check_ptr(addr)?; let path = clone_cstring_safely(unsafe { (&*addr).sun_path.as_ptr() })? .to_string_lossy() .into_owned(); diff --git a/src/libos/src/util/mem_util.rs b/src/libos/src/util/mem_util.rs index f04efc40..b38f643f 100644 --- a/src/libos/src/util/mem_util.rs +++ b/src/libos/src/util/mem_util.rs @@ -1,6 +1,8 @@ use super::*; use std::ffi::{CStr, CString}; +use std::mem::size_of; use std::ptr; +use vm::VMRange; /// Memory utilities that deals with primitive types passed from user process /// running inside enclave @@ -9,38 +11,45 @@ pub mod from_user { /// Check the user pointer is within the readable memory range of the user process pub fn check_ptr(user_ptr: *const T) -> Result<()> { - if user_ptr.is_null() { - return_errno!(EINVAL, "Address 0 is invalid"); + if !is_inside_user_space(user_ptr as *const u8, size_of::()) { + return_errno!(EFAULT, "pointer is not in the user space"); } Ok(()) } /// Check the mutable user pointer is within the writable memory of the user process pub fn check_mut_ptr(user_ptr: *mut T) -> Result<()> { - if user_ptr.is_null() { - return_errno!(EINVAL, "Address 0 is invalid"); - } - Ok(()) + // The user space is both readable and writable on SGX1. + // TODO: Fine-tune the checking on SGX2. + check_ptr(user_ptr) } /// Check the readonly array is within the readable memory of the user process pub fn check_array(user_buf: *const T, count: usize) -> Result<()> { - check_ptr(user_buf); + if !is_inside_user_space(user_buf as *const u8, count * size_of::()) { + return_errno!(EFAULT, "the whole buffer is not in the user space"); + } Ok(()) } /// Check the mutable array is within the writable memory of the user process pub fn check_mut_array(user_buf: *mut T, count: usize) -> Result<()> { - check_mut_ptr(user_buf); - Ok(()) + // The user space is both readable and writable on SGX1. + // TODO: Fine-tune the checking on SGX2. + check_array(user_buf, count) } /// Clone a C-string from the user process safely pub fn clone_cstring_safely(out_ptr: *const c_char) -> Result { - check_ptr(out_ptr)?; - // TODO: using from_ptr directly is not safe + if out_ptr.is_null() { + return_errno!(EINVAL, "NULL address is invalid"); + } + let cstr = unsafe { CStr::from_ptr(out_ptr) }; let cstring = CString::from(cstr); + if !is_inside_user_space(out_ptr as *const u8, cstring.as_bytes().len()) { + return_errno!(EFAULT, "the whole buffer is not in the user space"); + } Ok(cstring) } @@ -55,13 +64,13 @@ pub mod from_user { let mut user_ptr = user_ptr; loop { - check_ptr(user_ptr); + check_ptr(user_ptr)?; + let cstr_ptr = { let cstr_ptr = unsafe { *user_ptr }; if cstr_ptr == ptr::null() { break; } - check_ptr(cstr_ptr); cstr_ptr }; let cstring = clone_cstring_safely(cstr_ptr)?; @@ -71,6 +80,20 @@ pub mod from_user { } Ok(cstrings) } + + /// Check if the provided buffer is within the current user space + /// + /// addr: the start address + /// len: the length in byte + fn is_inside_user_space(addr: *const u8, len: usize) -> bool { + let current = current!(); + let current_vm = current.vm().lock().unwrap(); + let user_range = current_vm.get_process_range(); + let ur_start = user_range.start(); + let ur_end = user_range.end(); + let addr_start = addr as usize; + addr_start >= ur_start && addr_start < ur_end && ur_end - addr_start >= len + } } /// Memory utilities that deals with primitive types passed from outside the enclave @@ -78,34 +101,44 @@ pub mod from_untrusted { use super::*; /// Check the untrusted pointer is outside the enclave - // TODO: implement this! pub fn check_ptr(out_ptr: *const T) -> Result<()> { + if !sgx_trts::trts::rsgx_raw_is_outside_enclave(out_ptr as *const u8, size_of::()) { + return_errno!(EFAULT, "the pointer is not outside enclave"); + } Ok(()) } /// Check the untrusted array is outside the enclave - // TODO: implement this! pub fn check_array(out_ptr: *const T, count: usize) -> Result<()> { + if !sgx_trts::trts::rsgx_raw_is_outside_enclave( + out_ptr as *const u8, + count * size_of::(), + ) { + return_errno!(EFAULT, "the whole buffer is not outside enclave"); + } Ok(()) } /// Clone a C-string from outside the enclave - // TODO: strict check! pub fn clone_cstring_safely(out_ptr: *const c_char) -> Result { - check_ptr(out_ptr)?; if out_ptr.is_null() { - return_errno!(EINVAL, "null ptr"); + return_errno!(EINVAL, "NULL address is invalid"); } - // TODO: using from_ptr directly is not safe + let cstr = unsafe { CStr::from_ptr(out_ptr) }; let cstring = CString::from(cstr); + if !sgx_trts::trts::rsgx_raw_is_outside_enclave( + out_ptr as *const u8, + cstring.as_bytes().len(), + ) { + return_errno!(EFAULT, "the string is not outside enclave"); + } Ok(cstring) } /// Clone a C-string array (const char*[]) from outside the enclave /// /// This array must be ended with a NULL pointer. - // TODO: strict check! pub fn clone_cstrings_safely(out_ptr: *const *const c_char) -> Result> { let mut cstrings = Vec::new(); if out_ptr == ptr::null() { @@ -114,13 +147,13 @@ pub mod from_untrusted { let mut out_ptr = out_ptr; loop { - check_ptr(out_ptr); + check_ptr(out_ptr)?; + let cstr_ptr = { let cstr_ptr = unsafe { *out_ptr }; if cstr_ptr == ptr::null() { break; } - check_ptr(cstr_ptr); cstr_ptr }; let cstring = clone_cstring_safely(cstr_ptr)?;