Fix REST /set endpoint allowing overwrites of unowned documents

CHANGELOG.md

@@ -4,6 +4,7 @@
 
 ### Bug Fixes
 
+- Fix REST `/set` endpoint allowing a client to overwrite documents they do not own. When a `queryModifier` was configured, the handler only validated that the client-provided (new) document state matched the modifier but never checked the existing server document. An authenticated user could therefore 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.
 - Fix invalid CORS response when the server is configured with the default `cors: '*'`. The express adapter always sends `Access-Control-Allow-Credentials: true`, but combining that with `Access-Control-Allow-Origin: *` is rejected by browsers per the CORS spec, so credentialed (cookie/auth-header) requests from any cross-origin client would fail. The adapter now reflects the request `Origin` back when `cors` is `'*'`, keeping the "allow from anywhere" semantics while staying compatible with credentials.
 - Fix REST endpoint `/set` allowing clients to populate `serverOnlyFields` when inserting NEW documents. Updates to existing documents already stripped client-supplied values for these fields, but the insert path passed the client document straight to `RxCollection.insert()`, so a client could persist arbitrary values into fields that are documented as server-only. The handler now strips server-only fields from the client document before inserting, matching the documented contract that clients cannot do writes where one of the `serverOnlyFields` is set.
 - Fix replication pull URL not URL-encoding the checkpoint `id`. When a document's primary key contained URL-reserved characters (for example `&`, `#`, `=`), the URL was parsed incorrectly on the server, causing the checkpoint to be truncated. With `batchSize: 1` this could make the pull loop never advance past such a document. The client now encodes the `id` with `encodeURIComponent`.

src/plugins/server/endpoint-rest.ts

@@ -232,6 +232,15 @@ export class RxServerRestEndpoint<ServerAppType, AuthType, RxDocType> implements
                         );
                         promises.push(this.collection.insert(mergedDocData).catch(err => onWriteError(err, mergedDocData)));
                     } else {
+                        // The user must also be allowed to access the existing document.
+                        // Without this check, a client could overwrite arbitrary
+                        // documents by sending a write whose new state matches the
+                        // queryModifier while targeting a foreign document's primary.
+                        const isExistingDocAllowed = docDataMatcherWrite(doc.toJSON(true) as any);
+                        if (!isExistingDocAllowed) {
+                            adapter.closeConnection(res, 403, 'Forbidden');
+                            return;
+                        }
                         const isAllowed = this.changeValidator(authData, {
                             newDocumentState: removeServerOnlyFields(docData as any),
                             assumedMasterState: removeServerOnlyFields(doc.toJSON(true))

test/unit/endpoint-rest.test.ts

@@ -517,6 +517,56 @@ describe('endpoint-rest.test.ts', () => {
                 'error'
             );
 
+            await col.database.close();
+        });
+        it('should not allow overwriting a document the user does not own', async () => {
+            const col = await humansCollection.create(0);
+
+            // Insert a document owned by another user (bob).
+            const bobDoc = await col.insert(schemaObjects.humanData('bob-doc', 30, 'bob'));
+            const bobLastNameBefore = bobDoc.lastName;
+            const bobAgeBefore = bobDoc.age;
+
+            const port = await nextPort();
+            const server = await createRxServer({
+                adapter: TEST_SERVER_ADAPTER,
+                database: col.database,
+                authHandler,
+                port
+            });
+            const endpoint = await server.addRestEndpoint({
+                name: randomToken(10),
+                collection: col,
+                queryModifier
+            });
+            await server.start();
+
+            // Alice connects and tries to take over bob's document by sending a
+            // write with bob's passportId but alice's firstName so it passes the
+            // queryModifier check.
+            const client = createRestClient<HumanDocumentType>('http://localhost:' + port + '/' + endpoint.urlPath, headers);
+
+            const maliciousDoc: HumanDocumentType = {
+                passportId: 'bob-doc',
+                firstName: headers.userid,
+                lastName: 'hacked',
+                age: 99
+            };
+
+            let errorThrown = false;
+            try {
+                await client.set([maliciousDoc]);
+            } catch (err) {
+                errorThrown = true;
+            }
+            assert.ok(errorThrown, 'server must reject the overwrite attempt');
+
+            // Bob's document must still be intact.
+            const bobDocAfter = await col.findOne('bob-doc').exec(true);
+            assert.strictEqual(bobDocAfter.firstName, 'bob');
+            assert.strictEqual(bobDocAfter.lastName, bobLastNameBefore);
+            assert.strictEqual(bobDocAfter.age, bobAgeBefore);
+
             await col.database.close();
         });
     });

test/unit/perf-rest-set.ts

@@ -0,0 +1,109 @@
+/**
+ * Micro-benchmark for the REST /set endpoint.
+ * Measures the wall-time of bulk /set requests which hit the path
+ * that was changed by the overwrite-protection fix.
+ *
+ * Run with:
+ *   npx ts-node --esm ./test/unit/perf-rest-set.ts
+ * or
+ *   npx mocha --require ts-node/register ./test/unit/perf-rest-set.ts --exit
+ */
+import {
+    addRxPlugin,
+    randomToken
+} from 'rxdb/plugins/core';
+import { RxDBDevModePlugin } from 'rxdb/plugins/dev-mode';
+import {
+    createRxServer
+} from '../../plugins/server';
+import {
+    createRestClient
+} from '../../plugins/client-rest';
+import {
+    humansCollection,
+    HumanDocumentType
+} from 'rxdb/plugins/test-utils';
+import { nextPort } from './test-helpers.ts';
+
+import './config.ts';
+import {
+    authHandler,
+    headers,
+    queryModifier
+} from './test-helpers.ts';
+import { TEST_SERVER_ADAPTER } from './config-server.test.ts';
+
+async function run() {
+    addRxPlugin(RxDBDevModePlugin);
+
+    const totalDocs = 500;
+    const batches = 5;
+    const perBatch = totalDocs / batches;
+
+    const col = await humansCollection.create(0);
+
+    // seed docs owned by the authenticated user so they pass the queryModifier
+    const seed: HumanDocumentType[] = [];
+    for (let i = 0; i < totalDocs; i++) {
+        seed.push({
+            passportId: 'doc-' + i,
+            firstName: headers.userid,
+            lastName: 'seed',
+            age: 20
+        });
+    }
+    await col.bulkInsert(seed);
+
+    const port = await nextPort();
+    const server = await createRxServer({
+        adapter: TEST_SERVER_ADAPTER,
+        database: col.database,
+        authHandler,
+        port
+    });
+    const endpoint = await server.addRestEndpoint({
+        name: randomToken(10),
+        collection: col,
+        queryModifier
+    });
+    await server.start();
+
+    const client = createRestClient<HumanDocumentType>(
+        'http://localhost:' + port + '/' + endpoint.urlPath,
+        headers
+    );
+
+    // warmup
+    await client.set([{
+        passportId: 'doc-0',
+        firstName: headers.userid,
+        lastName: 'warm',
+        age: 1
+    }]);
+
+    const start = Date.now();
+    for (let b = 0; b < batches; b++) {
+        const batch: HumanDocumentType[] = [];
+        for (let i = 0; i < perBatch; i++) {
+            const idx = b * perBatch + i;
+            batch.push({
+                passportId: 'doc-' + idx,
+                firstName: headers.userid,
+                lastName: 'u' + b,
+                age: 30 + (idx % 10)
+            });
+        }
+        await client.set(batch);
+    }
+    const ms = Date.now() - start;
+
+    console.log('# perf-rest-set: updated ' + totalDocs + ' docs in ' + ms + 'ms (' + (totalDocs / (ms / 1000)).toFixed(1) + ' docs/sec)');
+
+    await server.close();
+    await col.database.close();
+}
+
+run().then(
+    () => process.exit(0),
+    err => { console.error(err); process.exit(1); }
+);