Skip to content

🔒️ Check ops from the server against the json0 path guard - #723

Draft
alecgibson wants to merge 1 commit into
fix-json0-op-path-guard-bypassfrom
fix-client-json0-path-guard
Draft

🔒️ Check ops from the server against the json0 path guard#723
alecgibson wants to merge 1 commit into
fix-json0-op-path-guard-bypassfrom
fix-client-json0-path-guard

Conversation

@alecgibson

Copy link
Copy Markdown
Collaborator

Closes #721

ot.js refuses to apply a json0 op whose path contains a segment inherited from Object.prototype. Doc has no equivalent check: _otApply() hands op data straight to this.type.apply(), so the guard protects the server's own Object.prototype, and nothing protects a client's.

That matters for documents whose history predates the guard, since a committed __proto__ op is replayed by every client that fetches the document, and for a hostile or compromised server, or anything else that can put a frame on the socket. The write is silent: doc.data is unchanged, no error is emitted, and nothing in the client notices.

The type won't complain, so the check has to be explicit. This change adds it to _otApply(), which covers remote ops, fixup ops echoed back by the server, and the inverted op on rollback. Every call site already wraps _otApply() in a try/catch and hard rollbacks, so throwing fits the existing contract, and the error surfaces the way it does for any other op we can't apply.

Locally submitted ops are checked in _submit() rather than left to _otApply(), because _pushOp() runs first, and _tryCompose() applies the op to a pending create on the way past. By the time _otApply() sees the op, the prototype is already polluted, permanently. Erroring out of _submit() also means we call back with the error rather than tearing the document down and refetching it, matching what we already do for an op submitted to an uncreated document.

The check reuses the traversal added for GHSA-9rqw-j2q5-gg2g, which walks an op the way ot-json0 does, so the array-like and non-string path segment bypasses are closed on the client too. It deliberately reuses only the path check, not the shape check: ot-json0 quietly treats a non-array as a no-op and older versions of ShareDB committed those, so they exist in real op histories, and the client is on the replay side of that line.

Note this doesn't clean up documents that already have such an op in their history. Those ops stay in the database and keep failing on every replay.

🤖 Generated with Claude Code

Co-Authored-By: Claude noreply@anthropic.com

Closes #721

`ot.js` refuses to apply a json0 op whose path contains a segment
inherited from `Object.prototype`. `Doc` has no equivalent check:
`_otApply()` hands op data straight to `this.type.apply()`, so the guard
protects the server's own `Object.prototype`, and nothing protects a
client's.

That matters for documents whose history predates the guard, since a
committed `__proto__` op is replayed by every client that fetches the
document, and for a hostile or compromised server, or anything else that
can put a frame on the socket. The write is silent: `doc.data` is
unchanged, no error is emitted, and nothing in the client notices.

The type won't complain, so the check has to be explicit. This change
adds it to `_otApply()`, which covers remote ops, fixup ops echoed back
by the server, and the inverted op on rollback. Every call site already
wraps `_otApply()` in a try/catch and hard rollbacks, so throwing fits
the existing contract, and the error surfaces the way it does for any
other op we can't apply.

Locally submitted ops are checked in `_submit()` rather than left to
`_otApply()`, because `_pushOp()` runs first, and `_tryCompose()`
applies the op to a pending create on the way past. By the time
`_otApply()` sees the op, the prototype is already polluted,
permanently. Erroring out of `_submit()` also means we call back with
the error rather than tearing the document down and refetching it,
matching what we already do for an op submitted to an uncreated
document.

The check reuses the traversal added for GHSA-9rqw-j2q5-gg2g, which
walks an op the way `ot-json0` does, so the array-like and non-string
path segment bypasses are closed on the client too. It deliberately
reuses only the path check, not the shape check: `ot-json0` quietly
treats a non-array as a no-op and older versions of ShareDB committed
those, so they exist in real op histories, and the client is on the
replay side of that line.

Note this doesn't clean up documents that already have such an op in
their history. Those ops stay in the database and keep failing on every
replay.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant