From a2615a8cbbd129d6b6d40ae3dd2cec232d8c7d4b Mon Sep 17 00:00:00 2001 From: kyren Date: Sun, 5 Aug 2018 11:54:33 -0400 Subject: [PATCH] Fix for a soundness bug around scope, don't allow callback parameters to escape Also includes other fixes for compiletest_rs failures, and a small reorg of tests --- src/scope.rs | 2 + src/tests/mod.rs | 104 +----------------- src/tests/scope.rs | 100 +++++++++++++++++ tests/compile-fail/not_refunwindsafe.rs | 3 +- tests/compile-fail/ref_not_unwindsafe.rs | 3 +- tests/compile-fail/scope_callback_escape.rs | 22 ++++ tests/compile-fail/scope_callback_inner.rs | 22 ++++ ...scope_escape.rs => scope_handle_escape.rs} | 0 8 files changed, 152 insertions(+), 104 deletions(-) create mode 100644 src/tests/scope.rs create mode 100644 tests/compile-fail/scope_callback_escape.rs create mode 100644 tests/compile-fail/scope_callback_inner.rs rename tests/compile-fail/{scope_escape.rs => scope_handle_escape.rs} (100%) diff --git a/src/scope.rs b/src/scope.rs index 476b5c1..58f440f 100644 --- a/src/scope.rs +++ b/src/scope.rs @@ -61,6 +61,7 @@ impl<'scope> Scope<'scope> { A: FromLuaMulti<'callback>, R: ToLuaMulti<'callback>, F: 'scope + Fn(&'callback Lua, A) -> Result, + 'scope: 'callback, { unsafe { let f = Box::new(move |lua, args| { @@ -106,6 +107,7 @@ impl<'scope> Scope<'scope> { A: FromLuaMulti<'callback>, R: ToLuaMulti<'callback>, F: 'scope + FnMut(&'callback Lua, A) -> Result, + 'scope: 'callback, { let func = RefCell::new(func); self.create_function(move |lua, args| { diff --git a/src/tests/mod.rs b/src/tests/mod.rs index deed3bd..196bc45 100644 --- a/src/tests/mod.rs +++ b/src/tests/mod.rs @@ -1,21 +1,17 @@ mod function; +mod scope; mod string; mod table; mod thread; mod types; mod userdata; -use std::cell::Cell; use std::iter::FromIterator; use std::panic::catch_unwind; -use std::rc::Rc; use std::sync::Arc; use std::{error, fmt}; -use { - Error, ExternalError, Function, Lua, Nil, Result, String, Table, UserData, UserDataMethods, - Value, Variadic, -}; +use {Error, ExternalError, Function, Lua, Nil, Result, String, Table, UserData, Value, Variadic}; #[test] fn test_load() { @@ -620,102 +616,6 @@ fn test_mismatched_registry_key() { }; } -#[test] -fn scope_func() { - let lua = Lua::new(); - - let rc = Rc::new(Cell::new(0)); - lua.scope(|scope| { - let r = rc.clone(); - let f = scope - .create_function(move |_, ()| { - r.set(42); - Ok(()) - }) - .unwrap(); - lua.globals().set("bad", f.clone()).unwrap(); - f.call::<_, ()>(()).unwrap(); - assert_eq!(Rc::strong_count(&rc), 2); - }); - assert_eq!(rc.get(), 42); - assert_eq!(Rc::strong_count(&rc), 1); - - match lua - .globals() - .get::<_, Function>("bad") - .unwrap() - .call::<_, ()>(()) - { - Err(Error::CallbackError { .. }) => {} - r => panic!("improper return for destructed function: {:?}", r), - }; -} - -#[test] -fn scope_drop() { - let lua = Lua::new(); - - struct MyUserdata(Rc<()>); - impl UserData for MyUserdata { - fn add_methods(methods: &mut UserDataMethods) { - methods.add_method("method", |_, _, ()| Ok(())); - } - } - - let rc = Rc::new(()); - - lua.scope(|scope| { - lua.globals() - .set( - "test", - scope.create_userdata(MyUserdata(rc.clone())).unwrap(), - ) - .unwrap(); - assert_eq!(Rc::strong_count(&rc), 2); - }); - assert_eq!(Rc::strong_count(&rc), 1); - - match lua.exec::<()>("test:method()", None) { - Err(Error::CallbackError { .. }) => {} - r => panic!("improper return for destructed userdata: {:?}", r), - }; -} - -#[test] -fn scope_capture() { - let lua = Lua::new(); - - let mut i = 0; - lua.scope(|scope| { - scope - .create_function_mut(|_, ()| { - i = 42; - Ok(()) - }) - .unwrap() - .call::<_, ()>(()) - .unwrap(); - }); - assert_eq!(i, 42); -} - -#[test] -fn outer_lua_access() { - let lua = Lua::new(); - let table = lua.create_table().unwrap(); - lua.scope(|scope| { - scope - .create_function_mut(|_, ()| { - table.set("a", "b").unwrap(); - Ok(()) - }) - .unwrap() - .call::<_, ()>(()) - .unwrap(); - }); - assert_eq!(table.get::<_, String>("a").unwrap(), "b"); -} - #[test] fn too_many_returns() { let lua = Lua::new(); diff --git a/src/tests/scope.rs b/src/tests/scope.rs new file mode 100644 index 0000000..1e4c255 --- /dev/null +++ b/src/tests/scope.rs @@ -0,0 +1,100 @@ +use std::cell::Cell; +use std::rc::Rc; + +use {Error, Function, Lua, String, UserData, UserDataMethods}; + +#[test] +fn scope_func() { + let lua = Lua::new(); + + let rc = Rc::new(Cell::new(0)); + lua.scope(|scope| { + let r = rc.clone(); + let f = scope + .create_function(move |_, ()| { + r.set(42); + Ok(()) + }) + .unwrap(); + lua.globals().set("bad", f.clone()).unwrap(); + f.call::<_, ()>(()).unwrap(); + assert_eq!(Rc::strong_count(&rc), 2); + }); + assert_eq!(rc.get(), 42); + assert_eq!(Rc::strong_count(&rc), 1); + + match lua + .globals() + .get::<_, Function>("bad") + .unwrap() + .call::<_, ()>(()) + { + Err(Error::CallbackError { .. }) => {} + r => panic!("improper return for destructed function: {:?}", r), + }; +} + +#[test] +fn scope_drop() { + let lua = Lua::new(); + + struct MyUserdata(Rc<()>); + impl UserData for MyUserdata { + fn add_methods(methods: &mut UserDataMethods) { + methods.add_method("method", |_, _, ()| Ok(())); + } + } + + let rc = Rc::new(()); + + lua.scope(|scope| { + lua.globals() + .set( + "test", + scope.create_userdata(MyUserdata(rc.clone())).unwrap(), + ) + .unwrap(); + assert_eq!(Rc::strong_count(&rc), 2); + }); + assert_eq!(Rc::strong_count(&rc), 1); + + match lua.exec::<()>("test:method()", None) { + Err(Error::CallbackError { .. }) => {} + r => panic!("improper return for destructed userdata: {:?}", r), + }; +} + +#[test] +fn scope_capture() { + let lua = Lua::new(); + + let mut i = 0; + lua.scope(|scope| { + scope + .create_function_mut(|_, ()| { + i = 42; + Ok(()) + }) + .unwrap() + .call::<_, ()>(()) + .unwrap(); + }); + assert_eq!(i, 42); +} + +#[test] +fn outer_lua_access() { + let lua = Lua::new(); + let table = lua.create_table().unwrap(); + lua.scope(|scope| { + scope + .create_function_mut(|_, ()| { + table.set("a", "b").unwrap(); + Ok(()) + }) + .unwrap() + .call::<_, ()>(()) + .unwrap(); + }); + assert_eq!(table.get::<_, String>("a").unwrap(), "b"); +} diff --git a/tests/compile-fail/not_refunwindsafe.rs b/tests/compile-fail/not_refunwindsafe.rs index 08de5de..55a3189 100644 --- a/tests/compile-fail/not_refunwindsafe.rs +++ b/tests/compile-fail/not_refunwindsafe.rs @@ -7,5 +7,6 @@ use rlua::*; fn main() { let lua = Lua::new(); let _ = catch_unwind(|| lua.create_table().unwrap()); - //~^ error: the trait bound `std::cell::UnsafeCell<()>: std::panic::RefUnwindSafe` is not satisfied in `rlua::Lua` + //~^ error: the type `std::cell::UnsafeCell<()>` may contain interior mutability and a reference + // may not be safely transferrable across a catch_unwind boundary } diff --git a/tests/compile-fail/ref_not_unwindsafe.rs b/tests/compile-fail/ref_not_unwindsafe.rs index 5debd16..19d64a6 100644 --- a/tests/compile-fail/ref_not_unwindsafe.rs +++ b/tests/compile-fail/ref_not_unwindsafe.rs @@ -8,5 +8,6 @@ fn main() { let lua = Lua::new(); let table = lua.create_table().unwrap(); let _ = catch_unwind(move || table.set("a", "b").unwrap()); - //~^ error: the trait bound `std::cell::UnsafeCell<()>: std::panic::RefUnwindSafe` is not satisfied in `rlua::Lua` + //~^ error: the type `std::cell::UnsafeCell<()>` may contain interior mutability and a reference + // may not be safely transferrable across a catch_unwind boundary } diff --git a/tests/compile-fail/scope_callback_escape.rs b/tests/compile-fail/scope_callback_escape.rs new file mode 100644 index 0000000..2da9d61 --- /dev/null +++ b/tests/compile-fail/scope_callback_escape.rs @@ -0,0 +1,22 @@ +extern crate rlua; + +use rlua::*; + +fn main() { + struct Test { + field: i32, + } + + let lua = Lua::new(); + let mut outer: Option = None; + lua.scope(|scope| { + let f = scope + .create_function_mut(|_, t: Table| { + outer = Some(t); + //~^ error: `*outer` does not live long enough + Ok(()) + }) + .unwrap(); + f.call::<_, ()>(lua.create_table()).unwrap(); + }); +} diff --git a/tests/compile-fail/scope_callback_inner.rs b/tests/compile-fail/scope_callback_inner.rs new file mode 100644 index 0000000..6e95a9e --- /dev/null +++ b/tests/compile-fail/scope_callback_inner.rs @@ -0,0 +1,22 @@ +extern crate rlua; + +use rlua::*; + +fn main() { + struct Test { + field: i32, + } + + let lua = Lua::new(); + lua.scope(|scope| { + let mut inner: Option
= None; + let f = scope + .create_function_mut(|_, t: Table| { + inner = Some(t); + //~^ error: `inner` does not live long enough + Ok(()) + }) + .unwrap(); + f.call::<_, ()>(lua.create_table()).unwrap(); + }); +} diff --git a/tests/compile-fail/scope_escape.rs b/tests/compile-fail/scope_handle_escape.rs similarity index 100% rename from tests/compile-fail/scope_escape.rs rename to tests/compile-fail/scope_handle_escape.rs