frozenCopy's iterative walk has no bound, so a registrant accessor that generates without end turns a neighbour's read into a process OOM

Aperta
  • Qualcosa non va?
#2,112 0 commenti 0 reazioni 0 assegnatari Vedi su GitHub

I maintainer di solito rispondono entro 1 giorno

Nessuno ha ancora preso questa issue.

Valutazione

Difficoltà
5/5
Tempo stimato
Più di una settimana
Idoneità per principianti
38/100
Tipo di issue
Bug
Chiarezza
Abbastanza chiara
Stato di attività
Attiva
Stack tecnologico
javascript
Ambito
backend

Direzione di ricerca

Start by reading src/core/runtime/activation.js around frozenCopy and src/core/registry/verbs.js around validateVerb, then inspect the ownership tests and the 20000-deep test added by PR #2111. The acceptance test should register a guarded field with a fresh-object accessor and have a second plugin read it. Done means that read avoids heap exhaustion, either completing or refusing deliberately while naming the registrant, and finite declarations still copy and freeze completely.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Descrizione

Source: #2111 at 533bd7be051765c7e3ee8ab59bb1d2c34134d1ac
Location: src/core/runtime/activation.js:1094 (frozenCopy)
Severity: minor

Evidence

PR #2111 replaced frozenCopy's recursion with an iterative walk, which is
the right answer for the depth case it was written for (issue #2049). It also
removes the only bound the walk ever had. The recursion's stack ceiling was an
accidental bound, but it was a bound, and it terminated a walk that cannot
terminate on its own.

A registrant whose guarded field carries an accessor that returns a fresh
object on every read gives the walk an infinite source. There is no cycle, so
the seen WeakMap never terminates it. Measured through the real
ctx.verbs.get() path, one plugin registering and a second reading, with
--max-old-space-size=384:

const gen = () => ({ type: 'object', get properties() { return { nest: gen() } } })
A.verbs.register({ name: 'owner gen', tool: 'owner_gen', inputSchema: gen(), ... })
B.verbs.get('owner gen').inputSchema

base 32d00dea   READ THREW RangeError: Maximum call stack size exceeded
                after 9ms, rss=91MB - process alive, throw confined to the read

head 533bd7be   FATAL ERROR: Ineffective mark-compacts near heap limit
                Allocation failed - JavaScript heap out of memory
                process dumped core

On a default heap the same input grows at roughly 50MB/s, so the daemon runs
several GB up before it dies rather than raising in ~10ms.

validateVerb (src/core/registry/verbs.js:315) checks only
!inputSchema || typeof inputSchema !== 'object', so nothing rejects such a
declaration at registration.

pending itself is not the problem and is correctly bounded: measured peak is
2 x the undrained siblings along the current path, which is 1 pair for a
200k-deep chain, 18 pairs for a depth-17 binary tree, and hits its 2(N-1)
ceiling only for a star. On a finite source the walk costs what the copy costs.
The unboundedness comes from the source, not from the frontier.

Expected behavior

A neighbour reading a guarded field of a registration whose accessors generate
without end gets an outcome confined to that read, as issue #2049 required for
the depth case. Killing the process takes down every other plugin, every
source and every sink, and is attributable to nobody.

Why deferral is safe

Plugins run in-process with no sandbox, so a registrant that can declare a
self-generating accessor can already exhaust the heap directly; the route adds
no capability an attacker did not have. It needs an accessor, so no
plain-data declaration reaches it, and no in-tree plugin declares one. It is
also the accessor-hazard family this repo already tracks deliberately
(#1505, #1519, #1524, #1530, #1555, #1971, #1976, #2059, #2087, #2092, #2096),
of which the throwing-accessor half is unchanged by #2111: a getter that
throws propagates into the neighbour's read identically on the base and on the
head (measured). Only the generating half regressed.

Closing it means choosing a bound, which #2111 deliberately declined to do for
depth, on the reasoning that a threshold is invented and a truncated copy
presented as a whole declaration is a false statement. That reasoning is right
and it is why this is a separate decision rather than a line in that PR.

Acceptance condition

A test registering a guarded field whose accessor returns a fresh nested
object on every read, with a second plugin reading the narrowed view: the read
must not exhaust the heap, and must either complete or fail with a deliberate,
reported refusal that names the registrant. Finite declarations of any depth
must still copy and freeze completely, so the existing ownership tests and the
20000-deep test added by #2111 must still pass unchanged.

Evidenced behavioral defect; eligible for automatic repair.

Lingua principale
JavaScript
Stelle
11
Fork
0
Merge medio
2h 49m
PR unite (30g)
302

Preparare l'ambiente

Questo progetto non fornisce container di sviluppo, Dockerfile né guida per i contributori, quindi l'ambiente è a tuo carico: parti dal suo README e consulta la nostra guida al primo contributo per i passaggi generali.

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di hyparam/hypaware

Tutte le issue di hyparam/hypaware

Issue simili

Altre issue su JavaScript

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.