🔒️ Check ops from the server against the json0 path guard - #723
Draft
alecgibson wants to merge 1 commit into
Draft
🔒️ Check ops from the server against the json0 path guard#723alecgibson wants to merge 1 commit into
alecgibson wants to merge 1 commit into
Conversation
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #721
ot.jsrefuses to apply a json0 op whose path contains a segment inherited fromObject.prototype.Dochas no equivalent check:_otApply()hands op data straight tothis.type.apply(), so the guard protects the server's ownObject.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.datais 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-json0does, 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-json0quietly 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