CVE-2023-24833: Use After Free (CWE-416)
value: A use-after-free in BigIntPrimitive addition in Hermes prior to commit a6dcafe6ded8e61658b40f5699878cd19a481f80 could have been used by an attacker to leak raw data from Hermes VM’s heap. Note that this 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 reuses or references memory after it has been freed. At some point afterward, the memory may be allocated again and saved in another pointer, while the original pointer references a location somewhere within the new allocation. Any operations using the original pointer are no longer valid because the memory "belongs" to the code that operates on the new pointer.
CVSS v3 base score: 7.5
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;
})