From 568d53c9b1eb81b38aaccd904cce682f8a7d0cf0 Mon Sep 17 00:00:00 2001
From: Linus Groh <mail@linusgroh.de>
Date: Sat, 12 Sep 2020 10:22:36 +0100
Subject: [PATCH] LibJS: Check validity of computed_property_name() result
 before using it

This fixes two cases obj[expr] and obj[expr]() (MemberExpression and
CallExpression respectively) when expr throws an exception and results
in an empty value, causing a crash by passing the invalid PropertyName
created by computed_property_name() to Object::get() without checking it
first.

Fixes #3459.
---
 Libraries/LibJS/AST.cpp                           | 10 ++++++++--
 Libraries/LibJS/Tests/computed-property-throws.js |  8 ++++++++
 Libraries/LibJS/Tests/exception-in-catch-block.js |  2 +-
 3 files changed, 17 insertions(+), 3 deletions(-)
 create mode 100644 Libraries/LibJS/Tests/computed-property-throws.js

diff --git a/Libraries/LibJS/AST.cpp b/Libraries/LibJS/AST.cpp
index 19161b9749..3886950442 100644
--- a/Libraries/LibJS/AST.cpp
+++ b/Libraries/LibJS/AST.cpp
@@ -122,7 +122,10 @@ CallExpression::ThisAndCallee CallExpression::compute_this_and_callee(Interprete
         auto* this_value = is_super_property_lookup ? &interpreter.this_value(global_object).as_object() : lookup_target.to_object(interpreter, global_object);
         if (interpreter.exception())
             return {};
-        auto callee = lookup_target.to_object(interpreter, global_object)->get(member_expression.computed_property_name(interpreter, global_object)).value_or(js_undefined());
+        auto property_name = member_expression.computed_property_name(interpreter, global_object);
+        if (!property_name.is_valid())
+            return {};
+        auto callee = lookup_target.to_object(interpreter, global_object)->get(property_name).value_or(js_undefined());
         return { this_value, callee };
     }
     return { &global_object, m_callee->execute(interpreter, global_object) };
@@ -1589,7 +1592,10 @@ Value MemberExpression::execute(Interpreter& interpreter, GlobalObject& global_o
     auto* object_result = object_value.to_object(interpreter, global_object);
     if (interpreter.exception())
         return {};
-    return object_result->get(computed_property_name(interpreter, global_object)).value_or(js_undefined());
+    auto property_name = computed_property_name(interpreter, global_object);
+    if (!property_name.is_valid())
+        return {};
+    return object_result->get(property_name).value_or(js_undefined());
 }
 
 Value StringLiteral::execute(Interpreter& interpreter, GlobalObject&) const
diff --git a/Libraries/LibJS/Tests/computed-property-throws.js b/Libraries/LibJS/Tests/computed-property-throws.js
new file mode 100644
index 0000000000..9c999c50e7
--- /dev/null
+++ b/Libraries/LibJS/Tests/computed-property-throws.js
@@ -0,0 +1,8 @@
+test("Issue #3459, exception in computed property expression", () => {
+    expect(() => {
+        "foo"[bar];
+    }).toThrow(ReferenceError);
+    expect(() => {
+        "foo"[bar]();
+    }).toThrow(ReferenceError);
+});
diff --git a/Libraries/LibJS/Tests/exception-in-catch-block.js b/Libraries/LibJS/Tests/exception-in-catch-block.js
index f84ad4cb41..2a0fa6e1fc 100644
--- a/Libraries/LibJS/Tests/exception-in-catch-block.js
+++ b/Libraries/LibJS/Tests/exception-in-catch-block.js
@@ -1,4 +1,4 @@
-test("Issue #1992, exception thrown in catch {} block", () => {
+test("Issue #3437, exception thrown in catch {} block", () => {
     var tryHasBeenExecuted = false;
     var catchHasBeenExecuted = false;
     var finallyHasBeenExecuted = false;