perf_hooks: Histogram.DoImport reads SharedArrayBuffer-backed data without copying (TOCTOU)

未关闭
  • 哪里不对？
#67,152 0 条评论 0 个 reaction 已指派 0 人 在 GitHub 查看

维护者通常 1 天内回复

还没有人认领这个 Issue。

评估

难度
3/5
预计耗时
1-2 天
新手友好度
62/100
Issue 类型
缺陷
描述清晰度
描述清楚
活跃度
活跃
技术栈
cpp, javascript
领域
security

调研方向

Read HistogramImpl::DoImport in src/histogram.cc and compare its buffer handling with the SharedArrayBuffer copy in src/node_buffer.cc. Find the existing histogram import tests and check how they cover Uint8Array inputs. Done means SAB-backed input is copied before Histogram::Import parses it, with normal inputs retaining their behavior and tests covering the case.

由索引模型根据 Issue 内容生成。

描述

Summary

HistogramImpl::DoImport in src/histogram.cc accepts a Uint8Array and reads the underlying buffer directly during CBOR parsing. When the Uint8Array is backed by a SharedArrayBuffer, a concurrent Worker thread can mutate the buffer while parsing is in progress, causing a time-of-check/time-of-use (TOCTOU) race.

Affected code

src/histogram.cc — HistogramImpl::DoImport (around line 3202):

Local<Uint8Array> input = args[0].As<Uint8Array>();
auto backing = input->Buffer()->GetBackingStore();
const uint8_t* data =
    static_cast<const uint8_t*>(backing->Data()) + input->ByteOffset();
size_t len = input->ByteLength();
// ← no IsSharedArrayBuffer() check
auto histogram = Histogram::Import(data, len);  // reads data during multi-step CBOR parse

There is no check for input->Buffer()->IsSharedArrayBuffer(). The CBOR parser in Histogram::Import performs multi-step parsing over data without any snapshot or copy.

Scenario

// Worker A — triggers importHistogram with a SAB-backed view
const sab = new SharedArrayBuffer(exportedBytes.length);
const view = new Uint8Array(sab);
view.set(exportedBytes);
// SAB is shared with Worker B via workerData or postMessage
importHistogram(view);  // Worker A starts CBOR parse

// Worker B — running concurrently
const view2 = new Uint8Array(sab);  // same SAB
view2[criticalOffset] = 0xff;       // mutates header field mid-parse

The perf_hooks worker-import pattern in handleMetricsImportWorker-style code already uses this exact shape: the server creates a SAB, copies request data into it, and passes it to a Worker. If the main thread or another Worker modifies the SAB while the Worker is inside importHistogram, the CBOR parser sees inconsistent data.

Existing precedent

src/node_buffer.cc lines 587–592 already handles this case for Buffer.toString():

if (view->Buffer()->IsSharedArrayBuffer()) {
  data_copy = std::make_unique_for_overwrite<char[]>(length);
  memcpy(data_copy.get(), data_ptr + start, length);
  data_ptr = data_copy.get();
  start = 0;
}

Proposed fix

Copy SAB-backed data to a local buffer before passing to Histogram::Import, matching the node_buffer.cc pattern. This eliminates the race without changing behavior for normal (non-SAB) Uint8Array inputs.

Severity

Low. Exploitation requires the caller to intentionally share a SAB with the histogram import thread and mutate it during parsing. This is not remotely triggerable without cooperation from same-process JavaScript. It is a defense-in-depth hardening, not a CVE-eligible vulnerability.

主要语言
JavaScript
星标
123k
派生
39.3k
平均合并
4 天 15 小时
30 天内合并 PR
238

环境准备

从这里开始

  1. 先读完整个 Issue，再读项目的贡献指南。
  2. 在 Issue 下留言说明你要接手 —— 这能避免两个人做同样的事。
  3. Fork 仓库，在一个分支上完成修改。
  4. 提交 Pull Request，并在描述里引用这个 Issue 编号。

nodejs/node 的其他 Issue

查看 nodejs/node 的全部 Issue

相似的 Issue

更多 JavaScript Issue

把新 issue 发到你的邮箱

精选适合新手参与的 GitHub issue 摘要。