Browse Source

Correct pop_on_return behaviour (#1853)

This PR changes the following:

- Updates the value of `pop_on_return` after a catch as to prevent VM stack corruption
- Adds two test cases which demonstrate the issue and demonstrate that it has been fixed

I am unsure if it is possible to abuse the patch provided; one would need to catch from within an array initialisation without calling into another frame (e.g., with a lambda), which I don't think is possible.
pull/1865/head
Addison Crump 3 years ago
parent
commit
2c19c6a4b5
  1. 1
      boa_engine/src/vm/mod.rs
  2. 40
      boa_engine/src/vm/tests.rs

1
boa_engine/src/vm/mod.rs

@ -1382,6 +1382,7 @@ impl Context {
for _ in 0..self.vm.frame().pop_on_return {
self.vm.pop();
}
self.vm.frame_mut().pop_on_return = 0;
self.vm.frame_mut().pc = address as usize;
self.vm.frame_mut().catch.pop();
self.vm.frame_mut().finally_return = FinallyReturn::Err;

40
boa_engine/src/vm/tests.rs

@ -1,4 +1,4 @@
use crate::exec;
use crate::{exec, Context, JsValue};
#[test]
fn typeof_string() {
@ -27,3 +27,41 @@ fn basic_op() {
"#;
assert_eq!(&exec(basic_op), "3");
}
#[test]
fn try_catch_finally_from_init() {
// the initialisation of the array here emits a PopOnReturnAdd op
//
// here we test that the stack is not popped more than intended due to multiple catches in the
// same function, which could lead to VM stack corruption
let source = r#"
try {
[(() => {throw "h";})()];
} catch (x) {
throw "h";
} finally {
}
"#;
assert_eq!(Context::default().eval(source.as_bytes()), Err("h".into()));
}
#[test]
fn multiple_catches() {
// see explanation on `try_catch_finally_from_init`
let source = r#"
try {
try {
[(() => {throw "h";})()];
} catch (x) {
throw "h";
}
} catch (y) {
}
"#;
assert_eq!(
Context::default().eval(source.as_bytes()),
Ok(JsValue::Undefined)
);
}

Loading…
Cancel
Save