CVE-2021-24037: Use After Free (CWE-416)
value: A use after free in hermes, while emitting certain error messages, prior to commit d86e185e485b6330216dee8e854455c694e3a36e allows attackers to potentially execute arbitrary code via crafted JavaScript. Note that this is only exploitable if the application using Hermes permits evaluation of 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: 9.8
Published: 2021-06-15T22:15Z
Repository: https://github.com/facebook/hermes
Language: C++
Files changed: include/hermes/VM/StringView.h, lib/VM/Interpreter.cpp
Commit date: 2020-08-14 15:39:45 -0700
Commit message: Fix a bug in transient object property assignment and getUTF16Ref

Summary:
The returned `UTF16Ref` from `StringView::getUTF16Ref` can be invalidated by
appending more contents to the same allocator.
This case was encountered in `transientObjectPutErrorMessage`, resulting in
using free'd memory.

Reviewed By: tmikov

Differential Revision: D23034855

fbshipit-source-id: 4c25a5369934bf3bdfc5582385503f4b87de3792

Diff: diff --git a/include/hermes/VM/StringView.h b/include/hermes/VM/StringView.h
index d15f848ca..a5170b828 100644
--- a/include/hermes/VM/StringView.h
+++ b/include/hermes/VM/StringView.h
@@ -317,13 +317,13 @@ class StringView {
 
   /// \return a UTF16Ref that pointing at the beginning of the string.
   /// If the string is already UTF16, we return the pointer directly;
   /// otherwise (it's ASCII) we copy the string into the end of \p allocator,
   /// and \return a pointer to the beginning of this string in the allocator.
-  /// Note: \p allocator does not need to be empty when passed in. We always
-  /// append.
+  /// \pre allocator must be empty when passed in.
   UTF16Ref getUTF16Ref(llvh::SmallVectorImpl<char16_t> &allocator) const {
+    assert(allocator.empty() && "Shouldn't use a non-empty allocator");
     return getUTF16Ref(allocator, false);
   }
 
   /// Force copying the string into \p allocator, even though the string
   /// may have already been UTF16.
diff --git a/lib/VM/Interpreter.cpp b/lib/VM/Interpreter.cpp
index 9fc0eebd5..cf8a7b8db 100644
--- a/lib/VM/Interpreter.cpp
+++ b/lib/VM/Interpreter.cpp
@@ -443,15 +443,16 @@ transientObjectPutErrorMessage(Runtime *runtime, Handle<> base, SymbolID id) {
     valueAsString = std::move(*str);
   }
   StringView valueAsStringPrintable =
       StringPrimitive::createStringView(runtime, valueAsString);
 
-  SmallU16String<32> tmp;
+  SmallU16String<32> tmp1;
+  SmallU16String<32> tmp2;
   return runtime->raiseTypeError(
       TwineChar16("Cannot create property '") + propName + "' on " +
-      baseTypeAsString.getUTF16Ref(tmp) + " '" +
-      valueAsStringPrintable.getUTF16Ref(tmp) + "'");
+      baseTypeAsString.getUTF16Ref(tmp1) + " '" +
+      valueAsStringPrintable.getUTF16Ref(tmp2) + "'");
 }
 
 ExecutionStatus Interpreter::putByIdTransient_RJS(
     Runtime *runtime,
     Handle<> base,

Vulnerable code:   /// Note: \p allocator does not need to be empty when passed in. We always
  /// append.
  SmallU16String<32> tmp;
      baseTypeAsString.getUTF16Ref(tmp) + " '" +
      valueAsStringPrintable.getUTF16Ref(tmp) + "'");
Fixed code:   /// \pre allocator must be empty when passed in.
    assert(allocator.empty() && "Shouldn't use a non-empty allocator");
  SmallU16String<32> tmp1;
  SmallU16String<32> tmp2;
      baseTypeAsString.getUTF16Ref(tmp1) + " '" +
      valueAsStringPrintable.getUTF16Ref(tmp2) + "'");