CVE-2023-23556: Out-of-bounds Write (CWE-787)
value: An error in BigInt conversion to Number in Hermes prior to commit a6dcafe6ded8e61658b40f5699878cd19a481f80 could have been used by a malicious attacker to execute arbitrary code due to an out-of-bound write. Note that this bug is only exploitable in cases where Hermes is used to execute untrusted JavaScript. Hence, most React Native applications are not affected.
CWE description: The product writes data past the end, or before the beginning, of the intended buffer.
CVSS v3 base score: 9.8
Published: 2023-05-18T22:15Z
Repository: https://github.com/facebook/hermes
Language: C++
Files changed: lib/Support/BigIntSupport.cpp, lib/VM/JSLib/Array.cpp, lib/VM/Operations.cpp, test/hermes/regress-bigint-add-gc.js, test/hermes/regress-bigint-to-double-nan.js, test/hermes/regress-invalidated-descriptor-std-sort-model-swap.js
Commit date: 2023-02-02 12:07:13 -0800
Commit message: Re-sync with internal repository (#908)

Fixes for CVE-2023-23556, and CVE-2023-24833.

Co-authored-by: Facebook Community Bot <6422482+facebook-github-bot@users.noreply.github.com>
Diff: diff --git a/lib/Support/BigIntSupport.cpp b/lib/Support/BigIntSupport.cpp
index b4fac6586f..2741a14599 100644
--- a/lib/Support/BigIntSupport.cpp
+++ b/lib/Support/BigIntSupport.cpp
@@ -190,11 +190,15 @@ double toDouble(ImmutableBigIntRef src) {
   }
 
   const uint32_t numBits = src.numDigits * BigIntDigitSizeInBits;
   llvh::APInt tmp(numBits, llvh::makeArrayRef(src.digits, src.numDigits));
   constexpr bool kSigned = true;
-  return tmp.roundToDouble(kSigned);
+  double val = tmp.roundToDouble(kSigned);
+  // TODO(T142034196): APInt::roundToDouble doesn't implement proper conversion
+  // when the number has more than 2 digits; we need to implement this
+  // conversion.
+  return std::isnan(val) ? std::numeric_limits<double>::quiet_NaN() : val;
 }
 
 namespace {
 /// ES5.1 7.2
 /// Copied from Operations.h to preserve layering.
diff --git a/lib/VM/JSLib/Array.cpp b/lib/VM/JSLib/Array.cpp
index 2c6676d5ab..c343316020 100644
--- a/lib/VM/JSLib/Array.cpp
+++ b/lib/VM/JSLib/Array.cpp
@@ -1091,14 +1091,10 @@ class StandardSortModel : public SortModel {
 
     ComputedPropertyDescriptor aDesc;
     JSObject::getComputedPrimitiveDescriptor(
         obj_, runtime_, aHandle_, aDescObjHandle_, aTmpNameStorage_, aDesc);
 
-    ComputedPropertyDescriptor bDesc;
-    JSObject::getComputedPrimitiveDescriptor(
-        obj_, runtime_, bHandle_, bDescObjHandle_, bTmpNameStorage_, bDesc);
-
     if (aDescObjHandle_) {
       if (LLVM_LIKELY(!aDesc.flags.proxyObject)) {
         auto res = JSObject::getComputedPropertyValue_RJS(
             obj_,
             runtime_,
@@ -1133,10 +1129,15 @@ class StandardSortModel : public SortModel {
         } else {
           aDescObjHandle_ = nullptr;
         }
       }
     }
+
+    ComputedPropertyDescriptor bDesc;
+    JSObject::getComputedPrimitiveDescriptor(
+        obj_, runtime_, bHandle_, bDescObjHandle_, bTmpNameStorage_, bDesc);
+
     if (bDescObjHandle_) {
       if (LLVM_LIKELY(!bDesc.flags.proxyObject)) {
         auto res = JSObject::getComputedPropertyValue_RJS(
             obj_,
             runtime_,
diff --git a/lib/VM/Operations.cpp b/lib/VM/Operations.cpp
index 21b422e3fa..a9e5f78006 100644
--- a/lib/VM/Operations.cpp
+++ b/lib/VM/Operations.cpp
@@ -1270,12 +1270,12 @@ addOp_RJS(Runtime &runtime, Handle<> xHandle, Handle<> yHandle) {
         "Cannot convert ", yHandle, " to BigInt");
   }
 
   return BigIntPrimitive::add(
       runtime,
-      runtime.makeHandle(resX->getBigInt()),
-      runtime.makeHandle(resY->getBigInt()));
+      runtime.makeHandle(xPrim->getBigInt()),
+      runtime.makeHandle(yPrim->getBigInt()));
 }
 
 static const size_t MIN_RADIX = 2;
 static const size_t MAX_RADIX = 36;
 
diff --git a/test/hermes/regress-bigint-add-gc.js b/test/hermes/regress-bigint-add-gc.js
new file mode 100644
index 0000000000..5e9588b3a3
--- /dev/null
+++ b/test/hermes/regress-bigint-add-gc.js
@@ -0,0 +1,21 @@
+/**
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+// RUN: %hermes -non-strict -O %s | %FileCheck %s
+// RUN: %hermes -non-strict -O0 %s | %FileCheck %s
+
+const obj = {
+    toString: () => {
+        gc();
+        return 0n;
+    }
+};
+// trigger after obj.toString called and before addOp complete
+let trigger_point = 1n + obj;
+print(trigger_point);
+
+// CHECK: 1
diff --git a/test/hermes/regress-bigint-to-double-nan.js b/test/hermes/regress-bigint-to-double-nan.js
new file mode 100644
index 0000000000..d306b8d8e9
--- /dev/null
+++ b/test/hermes/regress-bigint-to-double-nan.js
@@ -0,0 +1,13 @@
+/**
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+// RUN: %hermes -non-strict -O %s | %FileCheck %s
+// RUN: %hermes -non-strict -O0 %s | %FileCheck %s
+
+// TODO: fix BigInt to number conversion (it should never result in NaN).
+print(Number(0xffffffffffffffffffffffffffffffffn));
+// CHECK: NaN
diff --git a/test/hermes/regress-invalidated-descriptor-std-sort-model-swap.js b/test/hermes/regress-invalidated-descriptor-std-sort-model-swap.js
new file mode 100644
index 0000000000..1f6dfee9d3
--- /dev/null
+++ b/test/hermes/regress-invalidated-descriptor-std-sort-model-swap.js
@@ -0,0 +1,20 @@
+/**
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+// RUN: %hermes -O %s
+
+var a = [0,1]
+a.sort(function(x,y){
+  a.__defineGetter__(1, function(){
+    delete a[0];
+    return 1;
+  });
+  a.__defineGetter__(0, function(){
+    return 1;
+  });
+  return -1;
+})

Vulnerable code:   return tmp.roundToDouble(kSigned);
    ComputedPropertyDescriptor bDesc;
    JSObject::getComputedPrimitiveDescriptor(
        obj_, runtime_, bHandle_, bDescObjHandle_, bTmpNameStorage_, bDesc);

      runtime.makeHandle(resX->getBigInt()),
      runtime.makeHandle(resY->getBigInt()));
Fixed code:   double val = tmp.roundToDouble(kSigned);
  // TODO(T142034196): APInt::roundToDouble doesn't implement proper conversion
  // when the number has more than 2 digits; we need to implement this
  // conversion.
  return std::isnan(val) ? std::numeric_limits<double>::quiet_NaN() : val;

    ComputedPropertyDescriptor bDesc;
    JSObject::getComputedPrimitiveDescriptor(
        obj_, runtime_, bHandle_, bDescObjHandle_, bTmpNameStorage_, bDesc);

      runtime.makeHandle(xPrim->getBigInt()),
      runtime.makeHandle(yPrim->getBigInt()));
/**
 * Copyright (c) Meta Platforms, Inc. and affiliates.
 *
 * This source code is licensed under the MIT license found in the
 * LICENSE file in the root directory of this source tree.
 */

// RUN: %hermes -non-strict -O %s | %FileCheck %s
// RUN: %hermes -non-strict -O0 %s | %FileCheck %s

const obj = {
    toString: () => {
        gc();
        return 0n;
    }
};
// trigger after obj.toString called and before addOp complete
let trigger_point = 1n + obj;
print(trigger_point);

// CHECK: 1
/**
 * Copyright (c) Meta Platforms, Inc. and affiliates.
 *
 * This source code is licensed under the MIT license found in the
 * LICENSE file in the root directory of this source tree.
 */

// RUN: %hermes -non-strict -O %s | %FileCheck %s
// RUN: %hermes -non-strict -O0 %s | %FileCheck %s

// TODO: fix BigInt to number conversion (it should never result in NaN).
print(Number(0xffffffffffffffffffffffffffffffffn));
// CHECK: NaN
/**
 * Copyright (c) Meta Platforms, Inc. and affiliates.
 *
 * This source code is licensed under the MIT license found in the
 * LICENSE file in the root directory of this source tree.
 */

// RUN: %hermes -O %s

var a = [0,1]
a.sort(function(x,y){
  a.__defineGetter__(1, function(){
    delete a[0];
    return 1;
  });
  a.__defineGetter__(0, function(){
    return 1;
  });
  return -1;
})