Repository navigation
fix(host): resolve wire ids from their table's own keys only - #1719
Conversation
The saved tables are plain objects, so `state.bots['__proto__']`,
`state.rooms['__proto__']`, `tasks['__proto__']`, `state.agents['__proto__']`,
the capability library, `state.invitations['__proto__']`, `state.members[...]`
and `state.grants[...]` answer `Object.prototype`: these lookups took an id or
name straight from the request and then wrote a field through it.
Reproduced on live main `aafb8d74`, one behaviour per site:
- `update-bot {id:'__proto__', role:'Intruder'}` answers `ok:true` and sets
`Object.prototype.role`, because `rt.botOf` answers `Object.prototype` and
`roster.updateBot` then assigns `bot.role`.
- `task-update {id:'__proto__', title:'Stolen title'}` writes
`Object.prototype.title` in `applyFields` before the later throw.
- `set-flags {id:'__proto__', pinned:true}` writes `Object.prototype.pinned`
through the third arm, `state.agents[id]`.
- `set-main {id:'__proto__'}` answers `ok:true` and persists `'__proto__'` into
`state.mainBotIds`.
- `capability-delete {name:'__proto__'}` answers `{"ok":true,"value":true}` for a
capability that does not exist (`constructor` likewise).
Security's advisory named three more of the same class, verified here the same
way rather than quoted:
- `revokeInvitation('__proto__')` answers `ok:true` and sets
`Object.prototype.revokedAt`, because `state.invitations[id]` resolves to
`Object.prototype` and the revoke assigns through it.
- `removeMember('__proto__')` answers `{"ok":true}` for a member that is not
there, because `members[id]` resolves to `Object.prototype`.
- `setGrants('__proto__', [...])` passes the `state.bots[id]` existence check
and writes the grants table (`state.grants['__proto__'] = list` replaces its
prototype), then answers the list instead of `Unknown Bot`.
One own-key read now feeds every one of them: `ownEntry(table, id)` in store.js
(ThinkFlowLab#1708 introduced it for the Project ids; exported here, imported by tasks.js,
capabilities.js, roster.js, member-auth.js and reach.js) guards `botOf`,
`roomOf`, tasks' `find`, the capability library lookups, the `state.agents` arm
of `setFlags`, the invitation lookup in `revokeInvitation`, the member lookup in
`removeMember`, `grantsOf` and `setGrants`' Bot lookup. `setMain` refuses an id
that is not a Bot and no request writes through a key that is not own. The suite
pins each site separately: with `ownEntry` mutated back to `table[id]` all five
original tests fail, reverting only `botOf`/`roomOf`, only `find`, or only the
capability lookup fails exactly the sites that read through it, and each of the
three advisory sites fails its own test when its lookup is reverted.
Fixes ThinkFlowLab#1711
Owner: Dev-3
Written-by: Dev-3
Reviewer: Birfy
Claim: 905dfa1417324c76b16922d9787eb7b4
Base: f8e4943
Docs-not-needed: internal id lookups only; the documented host behavior is unchanged (same refusals, no API, state shape or tool text change).
|
Verdict: PASS at dd0b94c Ran at this tip: Red first (hard requirement): with the six changed sources ( Scope ( Tier: The change: one exported Checks at this tip: Not run here: the full Fork exemption: head is |
Fixes #1711
Owner: Dev-3
Written-by: Dev-3
Reviewer: Birfy
Claim: 905dfa1417324c76b16922d9787eb7b4
Base: f8e4943
Tip: dd0b94c
Tree: 0e828944b265946030345d3f7f07aeb89e2d59aa
Branch: dev/3-own-keys-everywhere
What and why
state.bots,state.rooms,tasks,state.agents,state.capabilities.items,state.invitations,state.membersandstate.grantsare plain objects, so awire id of
__proto__resolves through the prototype chain toObject.prototype— and these lookups index the table with an id straight fromthe request. Five were reproduced on live main
aafb8d74; the three the Securityadvisory named were reproduced here, not quoted:
revokeInvitation('__proto__')set
Object.prototype.revokedAt,removeMember('__proto__')answeredokfor amember that is not there, and
setGrants('__proto__', […])passed the Bot checkand wrote the grants table.
The fix is one own-key read,
ownEntry(table, id)— the pair #1708 introducedfor the Project ids in
store.js(merged asa932752d), exported here andimported by
tasks.js,capabilities.js,roster.js,member-auth.jsandreach.js. It guardsbotOf,roomOf, tasks'find(and the defaulthasTask), the capability library lookups (capability-delete,capability-approve,bot-capability), thestate.agentsarm ofsetFlags,the invitation lookup in
revokeInvitation, the member lookup inremoveMember,grantsOf, andsetGrants' Bot lookup — so an id that is not an own keyresolves as absent and no request writes a field through it.
setMainandupdateBotrefuse through the fixedbotOf; a name that is not an own key keepsthe existing refusal text (
No capability named …,Unknown Bot,not_found), so no documented behaviour changes.What I ran
Red first on live main (
aafb8d74), one behaviour per site — the assertions readthe prototype itself (
Object.hasOwn(Object.prototype, key)andkey in {};never
Object.hasOwn({}, key), which is false in the polluted state too):update-bot … Object.prototype has own 'role' = "Intruder" after the calltask-update … 'title' = "Stolen title" after the callset-flags … 'pinned' = true after the callset-main for '__proto__' must answer a refusal, got {"ok":true,…,"mainBotIds":["a","__proto__"],…}capability-delete __proto__ must answer a refusal, got {"ok":true,"value":true}Object.prototype has own 'revokedAt' = 1793292006111 after the callremove of '__proto__' must be not_found, got {"ok":true}grants for '__proto__' must be refused, got a stored listMutation self-checks at this tip:
ownEntry→table[id]fails all fiveoriginal tests; reverting only
botOf/roomOffails 1/4/5, onlyfindfails 2,only the capability lookup fails 3, and each advisory site fails its own test
when its lookup is reverted (
revokedAt,not_found,grantsrespectively).Restored: 8/8 green.
TZ=UTC ARCH_DOCS_BASE=f8e4943ca50501ddc45cbeaabb215438984122ac npm run verifyexits 0: core 2017 tests, social 539, tree 125, timeline 129, workflow 279,
.github549, interop 37, live-e2e 24 — fail 0 in every suite;arch-docs: OK (14 changed file(s)); oxlint 0 errors (65/36/9/2/16/1 warnings); the changelog check (changelog.d/ is valid and CHANGELOG.md keeps its Unreleased heading) green;node scripts/ds-bot-patches.mjs checkprints its threeoklines. Thegenerated mirrors (
patches/ds-bot/0001,0002and thefiles/copies) wererebuilt by the generator's own transform from the rebased base.
Not run: the
dev/bench.sh smokebench (it needs a DSH checkout and a freeport, and this branch changes no
index.jswiring); no live instance wasupdated or restarted.
Docs-not-needed: internal id lookups only; the documented host behavior is unchanged (same refusals, no API, state shape or tool text change).
{ "pr": 1719, "claim": "Dev-3", "issue": 1711, "summary": "Eight host lookups indexed a plain state table with a wire id, so `__proto__` resolved to `Object.prototype`: three of them wrote a field through it (role, title, pinned) and the advisory added three more (revokedAt, member removal, grants). One exported `ownEntry` read — the pair #1708 added for the Project ids — now guards botOf, roomOf, tasks.find, the capability lookups, setFlags, the invitation and member lookups, grantsOf and setGrants. Behaviour and refusal text are unchanged for real ids.", "files": [ "core/src/host/store.js", "core/src/host/tasks.js", "core/src/host/capabilities.js", "core/src/host/roster.js", "core/src/host/member-auth.js", "core/src/host/reach.js", "core/test/own-keys.test.js", "changelog.d/1711-own-key-lookups.security.md", "patches/ds-bot/0001-core-fixes.patch", "patches/ds-bot/0002-flybot-api-host.patch" ], "checks": { "ran": "TZ=UTC ARCH_DOCS_BASE=f8e4943c npm run verify", "result": "green", "at": "fed258d846d087d989210f1a4bf0f673f275e37d" }, "next": "none — ready for review" }