From 2c19c6a4b5adab95e7286bbc9a1a9d9a7ef7e62c Mon Sep 17 00:00:00 2001
From: Addison Crump <me@addisoncrump.info>
Date: Wed, 23 Feb 2022 10:37:57 +0000
Subject: [PATCH] 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.
---
 boa_engine/src/vm/mod.rs   |  1 +
 boa_engine/src/vm/tests.rs | 40 +++++++++++++++++++++++++++++++++++++-
 2 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/boa_engine/src/vm/mod.rs b/boa_engine/src/vm/mod.rs
index 95bf42db83..d8593bbe38 100644
--- a/boa_engine/src/vm/mod.rs
+++ b/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;
diff --git a/boa_engine/src/vm/tests.rs b/boa_engine/src/vm/tests.rs
index 9deb58dd19..4afb7ea208 100644
--- a/boa_engine/src/vm/tests.rs
+++ b/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)
+    );
+}