facebook / facebook/hermes

[Security] BCProvider: unvalidated getLargeHeaderOffset() enables heap OOB read in bytecode parsing

Open
#2,023 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
JavaScript
Stars
11.3k
Forks
859
Avg merge
1h 30m
Merged PRs (30d)
3

Description

**Please handle via private channel** — I am reporting this here as a fallback since Hermes has no SECURITY.md. Meta's security team may reach me via GitHub (@evilgensec) or through the Meta Bug Bounty program.

## Summary

Two heap out-of-bounds read vulnerabilities in `BCProviderFromBuffer` allow a crafted `.hbc` bytecode file to trigger OOB memory reads during bytecode loading and execution.

## Bug 1: Unvalidated `getLargeHeaderOffset()` (HIGH)

**File:** `include/hermes/BCGen/HBC/BCProvider.h:459`, `lib/BCGen/HBC/BCProvider.cpp:658`, `lib/BCGen/HBC/BCProvider.cpp:725`

When a `SmallFuncHeader` has `overflowed=true`, `getLargeHeaderOffset()` (a 32-bit attacker-controlled value) is added directly to `bufferPtr_` without bounds checking:

```cpp
// BCProvider.h:458-460 — no bounds check on getLargeHeaderOffset()
auto large = reinterpret_cast(
bufferPtr_ + smallHeader.getLargeHeaderOffset());
return RuntimeFunctionHeader(large);
```

If `getLargeHeaderOffset() > fileLength`, `large` points past the bytecode buffer. All subsequent field reads (e.g., `getOffset()`, `getBytecodeSizeInBytes()`, `flags`) are heap OOB reads.

**Downstream impact:** `getBytecode(functionID)` at BCProvider.h:495 returns `bufferPtr_ + [OOB-read offset]`, causing the interpreter/JIT to execute arbitrary heap data as bytecode instructions.

## Bug 2: Unvalidated overflow string table index (MEDIUM)

**File:** `include/hermes/BCGen/HBC/BCProvider.h:482`

When a `SmallStringTableEntry` has `isOverflowed()=true`, `getOffset()` (up to 23 bits, attacker-controlled) is used as an index into `overflowStringTableEntries_` without validation against `overflowStringCount`:

```cpp
// BCProvider.h:481-482 — getOffset() not bounded by overflowStringCount
if (LLVM_UNLIKELY(smallHeader.isOverflowed())) {
auto overflow = overflowStringTableEntries_[smallHeader.getOffset()]; // OOB
```

`llvh::ArrayRef::operator[]` performs no bounds checking. The returned OOB data is used as an `offset+length` to construct a `StringRef` pointing to arbitrary memory.

## Attack Surface

- React Native apps with OTA updates (e.g., CodePush) loading `.hbc` bundles
- Any app loading bytecode from external/untrusted sources
- MITM on OTA update channel → craft malicious `.hbc` → OOB reads in user device

## Suggested Fix

Validate `getLargeHeaderOffset()` against `fileLength - sizeof(FunctionHeader)` before use. Validate `smallHeader.getOffset()` against `overflowStringTableEntries_.size()` before indexing.

---
*Reported by evilgensec. Full technical details available privately.*

Contributor guide

Open the contributing guide

Research direction

Read the BCProvider paths in include/hermes/BCGen/HBC/BCProvider.h and lib/BCGen/HBC/BCProvider.cpp, especially getLargeHeaderOffset(), getBytecode(), and the overflow string-table lookup. Trace how malformed .hbc values reach pointer arithmetic and array indexing. Done means both attacker-controlled offsets are rejected before out-of-bounds reads occur, with behavior checked against malformed bytecode inputs.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
compilers, security
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.