Validate edit events come from the same user

.eslintrc.json

@@ -1,6 +1,4 @@
 {
-  "parser": "@typescript-eslint/parser",
-  "plugins": ["@typescript-eslint"],
   "parserOptions": {
     "ecmaVersion": 9,
     "ecmaFeatures": {
@@ -11,7 +9,6 @@
     "node": true,
     "jasmine": true
   },
-  "extends": ["plugin:@typescript-eslint/recommended"],
   "rules": {
     "consistent-return": "error",
     "curly": "error",
@@ -81,5 +78,13 @@
     "@typescript-eslint/explicit-module-boundary-types": "off",
     "@typescript-eslint/no-explicit-any": "warn",
     "@typescript-eslint/no-shadow": ["error"]
-  }
-}
+  },
+  "overrides": [
+    {
+      "files": ["*.ts"],
+      "parser": "@typescript-eslint/parser",
+      "plugins": ["@typescript-eslint"],
+      "extends": ["plugin:@typescript-eslint/recommended"]
+    }
+  ]
+}

changelog.d/329.feature

@@ -0,0 +1 @@
+Validate that the sender of a message edit matches the original sender.

spec/.eslintrc.json

@@ -0,0 +1,4 @@
+{
+  "strict": "off",
+  "no-var": "off"
+}

spec/integ/bridge.spec.js

@@ -10,8 +10,10 @@ const BOT_USER_ID = `@${BOT_LOCALPART}:${HS_DOMAIN}`;
 
 const TEST_USER_DB_PATH = __dirname + "/test-users.db";
 const TEST_ROOM_DB_PATH = __dirname + "/test-rooms.db";
+const TEST_EVENT_DB_PATH = __dirname + "/test-events.db";
 const UserBridgeStore = require("../..").UserBridgeStore;
 const RoomBridgeStore = require("../..").RoomBridgeStore;
+const EventBridgeStore = require("../..").EventBridgeStore;
 const MatrixUser = require("../..").MatrixUser;
 const RemoteUser = require("../..").RemoteUser;
 const MatrixRoom = require("../..").MatrixRoom;
@@ -23,7 +25,7 @@ const deferPromise = require("../../lib/utils/promiseutil").defer;
 
 describe("Bridge", function() {
     var bridge, bridgeCtrl, appService, clientFactory, appServiceRegistration;
-    var roomStore, userStore, clients;
+    var roomStore, userStore, eventStore, clients;
 
     beforeEach(
     /** @this */
@@ -97,16 +99,19 @@ describe("Bridge", function() {
 
         Promise.all([
             loadDatabase(TEST_USER_DB_PATH, UserBridgeStore),
-            loadDatabase(TEST_ROOM_DB_PATH, RoomBridgeStore)
-        ]).then(([userDb, roomDb]) => {
+            loadDatabase(TEST_ROOM_DB_PATH, RoomBridgeStore),
+            loadDatabase(TEST_EVENT_DB_PATH, EventBridgeStore)
+        ]).then(([userDb, roomDb, eventDb]) => {
             userStore = userDb;
             roomStore = roomDb;
+            eventStore = eventDb;
             bridge = new Bridge({
                 homeserverUrl: HS_URL,
                 domain: HS_DOMAIN,
                 registration: appServiceRegistration,
                 userStore: userDb,
                 roomStore: roomDb,
+                eventStore: eventDb,
                 controller: bridgeCtrl,
                 clientFactory: clientFactory
             });
@@ -129,6 +134,12 @@ describe("Bridge", function() {
         catch (e) {
             // do nothing
         }
+        try {
+            fs.unlinkSync(TEST_EVENT_DB_PATH);
+        }
+        catch (e) {
+            // do nothing
+        }
     });
 
     describe("onUserQuery", function() {
@@ -319,6 +330,137 @@ describe("Bridge", function() {
             expect(bridgeCtrl.onEvent).not.toHaveBeenCalled();
         });
 
+        describe('opts.eventValidation.validateEditSender', () => {
+            async function setupBridge(eventValidation) {
+                const bridge = new Bridge({
+                    homeserverUrl: HS_URL,
+                    domain: HS_DOMAIN,
+                    registration: appServiceRegistration,
+                    userStore: userStore,
+                    roomStore: roomStore,
+                    controller: bridgeCtrl,
+                    clientFactory: clientFactory,
+                    disableContext: true,
+                    eventValidation
+                });
+                await bridge.run(101, {}, appService);
+
+                return bridge;
+            }
+
+            function createMessageEditEvent(sender) {
+                const event = {
+                    content: {
+                        body: ' * my message edit',
+                        'm.new_content': { body: 'my message edit', msgtype: 'm.text' },
+                        'm.relates_to': { 
+                            event_id: '$ZrXenSQt4TbtHnMclrWNJdiP7SrRCSdl3tAYS81H2bs',
+                            rel_type: 'm.replace' 
+                        },
+                    msgtype: 'm.text'
+                    },
+                    event_id: '$tagvjsXZqBOBWtHijq2qg0Un-uqVunrFLxiJyOIVGQ8',
+                    room_id: '!dtJaPyDtsoOLTgJVmy:my.matrix.host',
+                    sender,
+                    type: 'm.room.message',
+                };
+
+                return event;
+            }
+
+            let botClient;
+            beforeEach(async () => {
+                botClient = clients["bot"];
+                botClient.getJoinedRoomMembers.and.returnValue(Promise.resolve({
+                    joined: {
+                        [botClient.credentials.userId]: {
+                            display_name: "bot"
+                        }
+                    }
+                }));
+
+                // Mock onEvent callback
+                bridgeCtrl.onEvent.and.callFake(function(req) { req.resolve(); });
+            });
+
+            describe('when enabled', () => {
+                beforeEach(async () => {
+                    bridge = await setupBridge({
+                        validateEditSender: {
+                            allowEventOnLookupFail: false,
+                        }
+                    })
+                });
+
+                it("should suppress the event if the edit is coming from a different person than the original message", async() => {
+                    const event = createMessageEditEvent('@root:my.matrix.host');
+
+                    botClient.fetchRoomEvent.and.returnValue(Promise.resolve({
+                        event_id: '$ZrXenSQt4TbtHnMclrWNJdiP7SrRCSdl3tAYS81H2bs',
+                        // The original message has different sender than the edit event
+                        sender: '@some-other-user:different.host',
+                    }));
+
+                    await appService.emit("event", event);
+                    expect(bridgeCtrl.onEvent).not.toHaveBeenCalled();
+                });
+
+                it("should emit event when the edit sender matches the original message sender", async() => {
+                    const event = createMessageEditEvent('@root:my.matrix.host');
+
+                    botClient.fetchRoomEvent.and.returnValue(Promise.resolve({
+                        event_id: '$ZrXenSQt4TbtHnMclrWNJdiP7SrRCSdl3tAYS81H2bs',
+                        // The original message sender is the same as the edit event
+                        sender: '@root:my.matrix.host',
+                    }));
+
+                    await appService.emit("event", event);
+                    expect(bridgeCtrl.onEvent).toHaveBeenCalled();
+                });
+            });
+
+            it('allowEventOnLookupFail=true should still emit event when failed to fetch original event', async() => {
+                const event = createMessageEditEvent('@root:my.matrix.host');
+
+                bridge = await setupBridge({
+                    validateEditSender: {
+                        // Option of interest for this test is here!
+                        allowEventOnLookupFail: true,
+                    }
+                })
+
+                const botClient = clients["bot"];
+                botClient.getJoinedRoomMembers.and.returnValue(Promise.resolve({
+                    joined: {
+                        [botClient.credentials.userId]: {
+                            display_name: "bot"
+                        }
+                    }
+                }));
+                botClient.fetchRoomEvent.and.returnValue(Promise.reject(new Error('Some problem fetching original event')));
+
+                await appService.emit("event", event);
+                expect(bridgeCtrl.onEvent).toHaveBeenCalled();
+            })
+
+            describe('when disabled', () => {
+                it("should emit event even when the edit sender does NOT match the original message sender", async() => {
+                    const event = createMessageEditEvent('@root:my.matrix.host');
+
+                    bridge = await setupBridge(undefined)
+
+                    botClient.fetchRoomEvent.and.returnValue(Promise.resolve({
+                        event_id: '$ZrXenSQt4TbtHnMclrWNJdiP7SrRCSdl3tAYS81H2bs',
+                        // The original message has different sender than the edit event
+                        sender: '@some-other-user:different.host',
+                    }));
+
+                    await appService.emit("event", event);
+                    expect(bridgeCtrl.onEvent).toHaveBeenCalled();
+                });
+            })
+        });
+
         it("should invoke the user-supplied onEvent function with the right args",
         function(done) {
             var event = {
@@ -488,6 +630,11 @@ describe("Bridge", function() {
             expect(bridge.getUserStore()).toEqual(userStore);
         });
 
+        it("should be able to getEventStore", async() => {
+            await bridge.run(101, {}, appService);
+            expect(bridge.getEventStore()).toEqual(eventStore);
+        });
+
         it("should be able to getRequestFactory", async() => {
             await bridge.run(101, {}, appService);
             expect(bridge.getRequestFactory()).toBeDefined();
@@ -792,7 +939,7 @@ function mkMockMatrixClient(uid) {
     var client = jasmine.createSpyObj(
         "MatrixClient", [
             "register", "joinRoom", "credentials", "createRoom", "setDisplayName",
-            "setAvatarUrl", "_http"
+            "setAvatarUrl", "fetchRoomEvent", "getJoinedRoomMembers", "_http"
         ]
     );
     // Shim requests to authedRequestWithPrefix to register() if it is

src/bridge.ts

@@ -250,6 +250,19 @@ export interface BridgeOpts {
         homeserverUrl: string;
         store: ClientEncryptionStore;
     };
+
+    eventValidation?: {
+        /**
+         * Should we validate that the sender of an edit matches the parent event.
+         */
+        validateEditSender?: {
+            /**
+             * If the parent edit event could not be found,
+             * should the event be rejected.
+             */
+            allowEventOnLookupFail: boolean;
+        };
+    };
 }
 
 interface VettedBridgeOpts {
@@ -382,6 +395,11 @@ interface VettedBridgeOpts {
         homeserverUrl: string;
         store: ClientEncryptionStore;
     };
+    eventValidation?: {
+        validateEditSender?: {
+            allowEventOnLookupFail: boolean;
+        };
+    };
 }
 
 export class Bridge {
@@ -462,6 +480,11 @@ export class Bridge {
             },
             logRequestOutcome: opts.logRequestOutcome ?? true,
             suppressEcho: opts.suppressEcho ?? true,
+            eventValidation: opts.hasOwnProperty("eventValidation") ? opts.eventValidation : {
+                validateEditSender: {
+                    allowEventOnLookupFail: false
+                }
+            }
         };
 
         this.queue = EventQueue.create(this.opts.queue, this.onConsume.bind(this));
@@ -1195,6 +1218,59 @@ export class Bridge {
         }
     }
 
+    /**
+     * Find a member for a given room. This method will fetch the joined members
+     * from the homeserver if the cache doesn't have it stored.
+     * @param preferBot Should we prefer the bot user over a ghost user
+     * @returns {Promise<string>} The userID of the member.
+     */
+    public async getAnyASMemberInRoom(roomId: string, preferBot = true) {
+        if (!this.registration) {
+            throw Error('Registration must be defined before you can call this');
+        }
+        let members = this.membershipCache.getMembersForRoom(roomId, "join");
+        if (!members) {
+            if (!this.appServiceBot) {
+                throw Error('AS Bot not defined yet');
+            }
+            members = Object.keys(await this.appServiceBot.getJoinedMembers(roomId));
+        }
+        if (preferBot && members?.includes(this.botUserId)) {
+            return this.botUserId;
+        }
+        const reg = this.registration;
+        return members.find((u) => reg.isUserMatch(u, false)) || null;
+    }
+
+    private async validateEditEvent(event: WeakEvent, parentEventId: string, allowEventOnLookupFail: boolean) {
+        try {
+            const roomMember = await this.getAnyASMemberInRoom(event.room_id);
+            if (!roomMember) {
+                throw Error('No member in room, cannot handle edit');
+            }
+            const relatedEvent = await this.getIntent(roomMember).getEvent(
+                event.room_id,
+                parentEventId,
+                true
+            );
+            // Only allow edits from the same sender
+            if (relatedEvent.sender !== event.sender) {
+                log.warn(
+                    `Rejecting ${event.event_id}: Message edit sender did NOT match the original message (${parentEventId})`
+                );
+                return false;
+            }
+        }
+        catch (ex) {
+            if (!allowEventOnLookupFail) {
+                log.warn(`Rejecting ${event.event_id}: Unable to fetch parent event ${parentEventId}`, ex);
+                return false;
+            }
+            log.warn(`Allowing event ${event.event_id}: Unable to fetch parent event ${parentEventId}`, ex);
+        }
+        return true;
+    }
+
     // returns a Promise for the request linked to this event for testing.
     private async onEvent(event: WeakEvent) {
         if (!this.registration) {
@@ -1214,6 +1290,22 @@ export class Bridge {
             return null;
         }
 
+        // eslint-disable-next-line camelcase
+        const relatesTo = event.content?.['m.relates_to'] as { event_id?: string; rel_type: "m.replace";}|undefined;
+        const editOptions = this.opts.eventValidation?.validateEditSender;
+
+        if (
+            event.type === 'm.room.message' &&
+            relatesTo?.rel_type === 'm.replace' &&
+            relatesTo.event_id &&
+            editOptions
+        ) {
+            // Event rejected.
+            if (!await this.validateEditEvent(event, relatesTo.event_id, editOptions.allowEventOnLookupFail)) {
+                return null;
+            }
+        }
+
         if (this.roomUpgradeHandler && this.opts.roomUpgradeOpts && this.appServiceBot) {
             // m.room.tombstone is the event that signals a room upgrade.
             if (event.type === "m.room.tombstone" && isCanonicalState && this.roomUpgradeHandler) {