Fix REST /set endpoint allowing overwrites of unowned documents#396
Merged
Fix REST /set endpoint allowing overwrites of unowned documents#396
Conversation
The REST /set endpoint only validated the client-provided (new) document state against the configured queryModifier. It never checked the existing server document, so an authenticated user could take over a foreign document by sending a write whose new state matched the modifier while targeting another user's primary key. The handler now also runs the query matcher against the existing server document and rejects the request with 403 Forbidden if it does not match, aligning the behavior with the replication /push endpoint.
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
2 participants
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.
This PR contains:
Describe the problem you have without this PR
The REST
/setendpoint had a security vulnerability where a client could overwrite documents they do not own. When aqueryModifierwas configured, the handler only validated that the client-provided (new) document state matched the modifier, but never checked whether the client was allowed to access the existing document being overwritten. This allowed a malicious client to take over arbitrary documents by crafting a write request whose new state passes thequeryModifierwhile targeting a foreign document's primary key.Changes
Source Code
docDataMatcherWrite(the same validator used for new documents) against the existing document's state to verify the client has permission to modify it.Tests
'should not allow overwriting a document the user does not own'that verifies:Performance Benchmarking
/setrequests on the modified code path. This ensures the security fix does not introduce significant performance regressions.Documentation
Test Plan
The new test case in
endpoint-rest.test.tsdirectly validates the fix by attempting an unauthorized document overwrite and verifying it is rejected. The benchmark inperf-rest-set.tscan be run to confirm no significant performance degradation was introduced.https://claude.ai/code/session_0187ohuAF69hYL4vrJwpUCEH