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

Abierto
  • ¿Algo no está bien?
#67,152 0 comentarios 0 reacciones 0 asignados Ver en GitHub

Los mantenedores suelen responder en 1 día

Nadie ha tomado este issue todavía.

Evaluación

Dificultad
3/5
Tiempo estimado
1-2 días
Aptitud para principiantes
62/100
Tipo de issue
Error
Claridad
Bien especificado
Estado de actividad
Activo
Stack tecnológico
cpp, javascript
Área
security

Línea de trabajo

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.

Escrito por el modelo de indexación a partir del texto del issue.

Descripción

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.

Lenguaje dominante
JavaScript
Estrellas
123k
Forks
39.3k
Merge medio
4 d 15 h
PR fusionados (30 d)
238

Preparar el entorno

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Más de nodejs/node

Todos los issues de nodejs/node

Issues similares

Más issues de JavaScript

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.