CVE-2020-1914 (C++, published 2020-10-08T19:15Z)
Description: value: A logic vulnerability when handling the SaveGeneratorLong instruction in Facebook Hermes prior to commit b2021df620824627f5a8c96615edbd1eb7fdddfc allows attackers to potentially read out of bounds or theoretically 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.
Severity: HIGH (CVSS v3 base score: 9.8)
Weakness: CWE-670 \u2014 Always-Incorrect Control Flow Implementation
The code contains a control flow path that does not reflect the algorithm that the path is intended to implement, leading to incorrect behavior any time this path is navigated.
Repository: https://github.com/facebook/hermes
Files changed:
include/hermes/VM/Interpreter.h
lib/VM/Interpreter-slowpaths.cpp
lib/VM/Interpreter.cpp
test/hermes/es6/generator.js
Commit message: Fix CVE-2020-1914 by using NEXTINST for SaveGeneratorLong

Summary:
If `SaveGeneratorLong` was emitted, it would accidentally jump to the
wrong next instruction, based on how long SaveGenerator was.

Make a callout function to handle the common case, and handle the dispatch
within each case of the interpreter loop.

Fixes CVE-2020-1914

Reviewed By: neildhar

Differential Revision: D24024242

fbshipit-source-id: 3bcb88daa740f0d50e91771a49eb212551ce8bd8


Vulnerable code:
        nextIP = IPADD(ip->iSaveGenerator.op1);
        goto doSaveGen;
        nextIP = IPADD(ip->iSaveGeneratorLong.op1);
        goto doSaveGen;
    doSaveGen : {
      auto *innerFn = vmcast<GeneratorInnerFunction>(
          runtime->getCurrentFrame().getCalleeClosure());

      innerFn->saveStack(runtime);
      innerFn->setNextIP(nextIP);
      innerFn->setState(GeneratorInnerFunction::State::SuspendedYield);
      ip = NEXTINST(SaveGenerator);
      DISPATCH;
    }


Fixed code:
  /// Suspend the generator function and yield to the caller.
  /// \param resumeIP Is the IP where the generator should resume from when it
  ///   is resumed.
  static void saveGenerator(
      Runtime *runtime,
      PinnedHermesValue *frameRegs,
      const Inst *resumeIP);

#include "hermes/VM/StackFrame-inline.h"
void Interpreter::saveGenerator(
    Runtime *runtime,
    PinnedHermesValue *frameRegs,
    const Inst *resumeIP) {
  auto *innerFn = vmcast<GeneratorInnerFunction>(FRAME.getCalleeClosure());
  innerFn->saveStack(runtime);
  innerFn->setNextIP(resumeIP);
  innerFn->setState(GeneratorInnerFunction::State::SuspendedYield);
}

/// \def DONT_CAPTURE_IP(expr)
/// \param expr A call expression to a function external to the interpreter. The
///   expression should not make any allocations and the IP should be set
///   immediately following this macro.
#define DONT_CAPTURE_IP(expr)      \
  do {                             \
    NoAllocScope noAlloc(runtime); \
    (void)expr;                    \
  } while (false)

        DONT_CAPTURE_IP(
            saveGenerator(runtime, frameRegs, IPADD(ip->iSaveGenerator.op1)));
        ip = NEXTINST(SaveGenerator);
        DISPATCH;
        DONT_CAPTURE_IP(saveGenerator(
            runtime, frameRegs, IPADD(ip->iSaveGeneratorLong.op1)));
        ip = NEXTINST(SaveGeneratorLong);
        DISPATCH;

// Make sure using SaveGeneratorLong works.
function* saveGeneratorLong() {
    yield* [1];
    // Waste some registers, to change SaveGenerator to SaveGeneratorLong.
    [].push(0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,
    0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,
    0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,
    0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,
    0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,
    0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,
    0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0);
}
print(saveGeneratorLong().next().value);
// CHECK-NEXT: 1