Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix UB in setting memory in emscripten EmEnv #2241

Merged
merged 2 commits into from
Apr 16, 2021
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
33 changes: 14 additions & 19 deletions lib/emscripten/src/env/unix/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,10 +150,11 @@ pub fn _gai_strerror(ctx: &EmEnv, ecode: i32) -> i32 {
let cstr = unsafe { std::ffi::CStr::from_ptr(libc::gai_strerror(ecode)) };
let bytes = cstr.to_bytes_with_nul();
let string_on_guest: WasmPtr<c_char, Array> = call_malloc_with_cast(ctx, bytes.len() as _);
let memory = ctx.memory(0);

let writer = unsafe {
string_on_guest
.deref_mut(ctx.memory(0), 0, bytes.len() as _)
.deref_mut(&memory, 0, bytes.len() as _)
.unwrap()
};
for (i, byte) in bytes.iter().enumerate() {
Expand All @@ -175,7 +176,7 @@ pub fn _getaddrinfo(
let memory = ctx.memory(0);
debug!(" => node = {}", unsafe {
node_ptr
.deref(memory)
.deref(&memory)
.map(|np| {
std::ffi::CStr::from_ptr(np as *const Cell<c_char> as *const c_char)
.to_string_lossy()
Expand All @@ -184,15 +185,15 @@ pub fn _getaddrinfo(
});
debug!(" => server_str = {}", unsafe {
service_str_ptr
.deref(memory)
.deref(&memory)
.map(|np| {
std::ffi::CStr::from_ptr(np as *const Cell<c_char> as *const c_char)
.to_string_lossy()
})
.unwrap_or(std::borrow::Cow::Borrowed("null"))
});

let hints = hints_ptr.deref(memory).map(|hints_memory| {
let hints = hints_ptr.deref(&memory).map(|hints_memory| {
let hints_guest = hints_memory.get();
addrinfo {
ai_flags: hints_guest.ai_flags,
Expand All @@ -212,11 +213,11 @@ pub fn _getaddrinfo(
let result = unsafe {
libc::getaddrinfo(
(node_ptr
.deref(memory)
.deref(&memory)
.map(|m| m as *const Cell<c_char> as *const c_char))
.unwrap_or(std::ptr::null()),
service_str_ptr
.deref(memory)
.deref(&memory)
.map(|m| m as *const Cell<c_char> as *const c_char)
.unwrap_or(std::ptr::null()),
hints
Expand Down Expand Up @@ -245,7 +246,7 @@ pub fn _getaddrinfo(

// connect list
if let Some(prev_guest) = previous_guest_node {
let mut pg = prev_guest.deref_mut(ctx.memory(0)).unwrap().get_mut();
let mut pg = prev_guest.deref_mut(&memory).unwrap().get_mut();
pg.ai_next = current_guest_node_ptr;
}

Expand All @@ -257,10 +258,7 @@ pub fn _getaddrinfo(
let host_sockaddr_ptr = (*current_host_node).ai_addr;
let guest_sockaddr_ptr: WasmPtr<EmSockAddr> =
call_malloc_with_cast(ctx, host_addrlen as _);
let guest_sockaddr = guest_sockaddr_ptr
.deref_mut(ctx.memory(0))
.unwrap()
.get_mut();
let guest_sockaddr = guest_sockaddr_ptr.deref_mut(&memory).unwrap().get_mut();

guest_sockaddr.sa_family = (*host_sockaddr_ptr).sa_family as i16;
guest_sockaddr.sa_data = (*host_sockaddr_ptr).sa_data;
Expand All @@ -277,9 +275,8 @@ pub fn _getaddrinfo(
let guest_canonname: WasmPtr<c_char, Array> =
call_malloc_with_cast(ctx, str_size as _);

let guest_canonname_writer = guest_canonname
.deref(ctx.memory(0), 0, str_size as _)
.unwrap();
let guest_canonname_writer =
guest_canonname.deref(&memory, 0, str_size as _).unwrap();
for (i, b) in canonname_bytes.into_iter().enumerate() {
guest_canonname_writer[i].set(*b as _)
}
Expand All @@ -290,10 +287,8 @@ pub fn _getaddrinfo(
}
};

let mut current_guest_node = current_guest_node_ptr
.deref_mut(ctx.memory(0))
.unwrap()
.get_mut();
let mut current_guest_node =
current_guest_node_ptr.deref_mut(&memory).unwrap().get_mut();
current_guest_node.ai_flags = (*current_host_node).ai_flags;
current_guest_node.ai_family = (*current_host_node).ai_family;
current_guest_node.ai_socktype = (*current_host_node).ai_socktype;
Expand All @@ -311,7 +306,7 @@ pub fn _getaddrinfo(
head_of_list.unwrap_or_else(|| WasmPtr::new(0))
};

res_val_ptr.deref(ctx.memory(0)).unwrap().set(head_of_list);
res_val_ptr.deref(&memory).unwrap().set(head_of_list);

0
}
20 changes: 13 additions & 7 deletions lib/emscripten/src/env/windows/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ extern "C" {
/// emscripten: _getenv // (name: *const char) -> *const c_char;
pub fn _getenv(ctx: &EmEnv, name: u32) -> u32 {
debug!("emscripten::_getenv");
let name_string = read_string_from_wasm(ctx.memory(0), name);
let memory = ctx.memory(0);
let name_string = read_string_from_wasm(&memory, name);
debug!("=> name({:?})", name_string);
let c_str = unsafe { getenv(name_string.as_ptr() as *const libc::c_char) };
if c_str.is_null() {
Expand All @@ -31,9 +32,10 @@ pub fn _getenv(ctx: &EmEnv, name: u32) -> u32 {
/// emscripten: _setenv // (name: *const char, name: *const value, overwrite: int);
pub fn _setenv(ctx: &EmEnv, name: u32, value: u32, _overwrite: u32) -> c_int {
debug!("emscripten::_setenv");
let memory = ctx.memory(0);
// setenv does not exist on windows, so we hack it with _putenv
let name = read_string_from_wasm(ctx.memory(0), name);
let value = read_string_from_wasm(ctx.memory(0), value);
let name = read_string_from_wasm(&memory, name);
let value = read_string_from_wasm(&memory, value);
let putenv_string = format!("{}={}", name, value);
let putenv_cstring = CString::new(putenv_string).unwrap();
let putenv_raw_ptr = putenv_cstring.as_ptr();
Expand All @@ -45,7 +47,8 @@ pub fn _setenv(ctx: &EmEnv, name: u32, value: u32, _overwrite: u32) -> c_int {
/// emscripten: _putenv // (name: *const char);
pub fn _putenv(ctx: &EmEnv, name: c_int) -> c_int {
debug!("emscripten::_putenv");
let name_addr = emscripten_memory_pointer!(ctx.memory(0), name) as *const c_char;
let memory = ctx.memory(0);
let name_addr = emscripten_memory_pointer!(&memory, name) as *const c_char;
debug!("=> name({:?})", unsafe {
std::ffi::CStr::from_ptr(name_addr)
});
Expand All @@ -55,7 +58,8 @@ pub fn _putenv(ctx: &EmEnv, name: c_int) -> c_int {
/// emscripten: _unsetenv // (name: *const char);
pub fn _unsetenv(ctx: &EmEnv, name: u32) -> c_int {
debug!("emscripten::_unsetenv");
let name = read_string_from_wasm(ctx.memory(0), name);
let memory = ctx.memory(0);
let name = read_string_from_wasm(&memory, name);
// no unsetenv on windows, so use putenv with an empty value
let unsetenv_string = format!("{}=", name);
let unsetenv_cstring = CString::new(unsetenv_string).unwrap();
Expand All @@ -69,6 +73,7 @@ pub fn _getpwnam(ctx: &EmEnv, name_ptr: c_int) -> c_int {
debug!("emscripten::_getpwnam {}", name_ptr);
#[cfg(not(feature = "debug"))]
let _ = name_ptr;
let memory = ctx.memory(0);

#[repr(C)]
struct GuestPasswd {
Expand All @@ -85,7 +90,7 @@ pub fn _getpwnam(ctx: &EmEnv, name_ptr: c_int) -> c_int {
unsafe {
let passwd_struct_offset = call_malloc(ctx, mem::size_of::<GuestPasswd>() as _);
let passwd_struct_ptr =
emscripten_memory_pointer!(ctx.memory(0), passwd_struct_offset) as *mut GuestPasswd;
emscripten_memory_pointer!(&memory, passwd_struct_offset) as *mut GuestPasswd;
(*passwd_struct_ptr).pw_name = 0;
(*passwd_struct_ptr).pw_passwd = 0;
(*passwd_struct_ptr).pw_gecos = 0;
Expand All @@ -103,6 +108,7 @@ pub fn _getgrnam(ctx: &EmEnv, name_ptr: c_int) -> c_int {
debug!("emscripten::_getgrnam {}", name_ptr);
#[cfg(not(feature = "debug"))]
let _ = name_ptr;
let memory = ctx.memory(0);

#[repr(C)]
struct GuestGroup {
Expand All @@ -116,7 +122,7 @@ pub fn _getgrnam(ctx: &EmEnv, name_ptr: c_int) -> c_int {
unsafe {
let group_struct_offset = call_malloc(ctx, mem::size_of::<GuestGroup>() as _);
let group_struct_ptr =
emscripten_memory_pointer!(ctx.memory(0), group_struct_offset) as *mut GuestGroup;
emscripten_memory_pointer!(&memory, group_struct_offset) as *mut GuestGroup;
(*group_struct_ptr).gr_name = 0;
(*group_struct_ptr).gr_passwd = 0;
(*group_struct_ptr).gr_gid = 0;
Expand Down
19 changes: 8 additions & 11 deletions lib/emscripten/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use lazy_static::lazy_static;
use std::collections::HashMap;
use std::f64;
use std::path::PathBuf;
use std::sync::{Arc, Mutex};
use std::sync::{Arc, Mutex, RwLock};
use wasmer::{
imports, namespace, Exports, Function, FunctionType, Global, ImportObject, Instance, LazyInit,
Memory, MemoryType, Module, NativeFunc, Pages, RuntimeError, Store, Table, TableType, Val,
Expand Down Expand Up @@ -71,7 +71,7 @@ pub use self::utils::{
#[derive(Clone)]
/// The environment provided to the Emscripten imports.
pub struct EmEnv {
memory: Arc<Option<Memory>>,
memory: Arc<RwLock<Option<Memory>>>,
data: Arc<Mutex<EmscriptenData>>,
}

Expand All @@ -86,21 +86,19 @@ impl WasmerEnv for EmEnv {
impl EmEnv {
pub fn new(data: &EmscriptenGlobalsData, mapped_dirs: HashMap<String, PathBuf>) -> Self {
Self {
memory: Arc::new(None),
memory: Arc::new(RwLock::new(None)),
data: Arc::new(Mutex::new(EmscriptenData::new(data.clone(), mapped_dirs))),
}
}

pub fn set_memory(&mut self, memory: Memory) {
let ptr = Arc::as_ptr(&self.memory) as *mut _;
unsafe {
*ptr = Some(memory);
}
let mut w = self.memory.write().unwrap();
*w = Some(memory);
}

/// Get a reference to the memory
pub fn memory(&self, _mem_idx: u32) -> &Memory {
(*self.memory).as_ref().unwrap()
pub fn memory(&self, _mem_idx: u32) -> Memory {
(&*self.memory.read().unwrap()).as_ref().cloned().unwrap()
}
}

Expand Down Expand Up @@ -477,8 +475,7 @@ impl EmscriptenGlobals {
minimum: table_min,
maximum: table_max,
};
// TODO: review init value
let table = Table::new(store, table_type, Val::null()).unwrap();
let table = Table::new(store, table_type, Val::FuncRef(None)).unwrap();

let data = {
let static_bump = STATIC_BUMP;
Expand Down
6 changes: 4 additions & 2 deletions lib/emscripten/src/syscalls/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -356,8 +356,9 @@ pub fn ___syscall183(ctx: &EmEnv, _which: c_int, mut varargs: VarArgs) -> i32 {
let path = get_current_directory(ctx);
let path_string = path.unwrap().display().to_string();
let len = path_string.len();
let memory = ctx.memory(0);

let buf_writer = buf_offset.deref(ctx.memory(0), 0, len as u32 + 1).unwrap();
let buf_writer = buf_offset.deref(&memory, 0, len as u32 + 1).unwrap();
for (i, byte) in path_string.bytes().enumerate() {
buf_writer[i].set(byte as _);
}
Expand Down Expand Up @@ -411,8 +412,9 @@ pub fn ___syscall140(ctx: &EmEnv, _which: i32, mut varargs: VarArgs) -> i32 {
let whence: i32 = varargs.get(ctx);
let offset = offset_low;
let ret = unsafe { lseek(fd, offset as _, whence) as i64 };
let memory = ctx.memory(0);

let result_ptr = result_ptr_value.deref(ctx.memory(0)).unwrap();
let result_ptr = result_ptr_value.deref(&memory).unwrap();
result_ptr.set(ret);

debug!(
Expand Down
Loading