Vulnerability: matrix-js-sdk can be tricked into disclosing E2EE room keys to a participating homeserver ### Impact

A logic error in the room key sharing functionality of matrix-js-sdk before 12.4.1 allows a malicious Matrix homeserver† participating in an encrypted room to steal room encryption keys from affected Matrix clients participating in that room. This allows the homeserver to decrypt end-to-end encrypted messages sent by affected clients.

† Or anyone with access to the account of the original recipient of an encrypted message.

Known clients affected (via their use of vulnerable versions of matrix-js-sdk):

- Element Web (1.8.2 and earlier)
- Element Desktop (1.8.2 and earlier)
- SchildiChat Web (1.7.32-sc1 and earlier)
- SchildiChat Desktop (1.7.32-sc1 and earlier)
- Cinny (1.2.0 and earlier)

### Patch

This was fixed in https://github.com/matrix-org/matrix-js-sdk/commit/894c24880da0e1cc81818f51c0db80e3c9fb2be9.

### Workarounds
To prevent a homeserver from being able to steal the room keys, vulnerable clients can be taken offline or signed out. If signing out, care should be taken to either set up Secure Backup or export E2E room keys in order to preserve access to past messages.
Vulnerability ID: GHSA-23cm-x6j7-6hq3
CWE: CWE-200
Severity score: 5.9
Project: https://github.com/matrix-org/matrix-js-sdk
Published: 2021-09-14
File: src/crypto/algorithms/megolm.ts
Commit message: Verify target device key on reshare
Commit author: RiotRobot

Vulnerable version (before fix):
 interface IPayload extends Partial<IMessage> { } /* eslint-enable camelcase */  /**  * @private  * @constructor interface IPayload extends Partial<IMessage> {  *  * @property {object} sharedWithDevices  *    devices with which we have shared the session key  *        userId -> {deviceId -> msgindex}  */ class OutboundSessionInfo {     public useCount = 0;     public creationTime: number;     public sharedWithDevices: Record<string, Record<string, number>> = {};     public blockedDevicesNotified: Record<string, Record<string, boolean>> = {};      constructor(public readonly sessionId: string, public readonly sharedHistory = false) { class OutboundSessionInfo {         return false;     }      public markSharedWithDevice(userId: string, deviceId: string, chainIndex: number): void {         if (!this.sharedWithDevices[userId]) {             this.sharedWithDevices[userId] = {};         }         this.sharedWithDevices[userId][deviceId] = chainIndex;     }      public markNotifiedBlockedDevice(userId: string, deviceId: string): void { class MegolmEncryption extends EncryptionAlgorithm {         payload: IPayload,     ): Promise<void> {         const contentMap = {};          const promises = [];         for (let i = 0; i < userDeviceMap.length; i++) { class MegolmEncryption extends EncryptionAlgorithm {             const userId = val.userId;             const deviceInfo = val.deviceInfo;             const deviceId = deviceInfo.deviceId;              if (!contentMap[userId]) {                 contentMap[userId] = {}; class MegolmEncryption extends EncryptionAlgorithm {                 for (const userId of Object.keys(contentMap)) {                     for (const deviceId of Object.keys(contentMap[userId])) {                         session.markSharedWithDevice(                             userId, deviceId, chainIndex,                         );                     }                 } class MegolmEncryption extends EncryptionAlgorithm {             logger.debug(`megolm session ${sessionId} never shared with user ${userId}`);             return;         }         const sentChainIndex = obSessionInfo.sharedWithDevices[userId][device.deviceId];         if (sentChainIndex === undefined) {             logger.debug(                 "megolm session ID " + sessionId + " never shared with device " +                 userId + ":" + device.deviceId,             );             return;         }          // get the key from the inbound session: the outbound one will already         // have been ratcheted to the next chain index.         const key = await this.olmDevice.getInboundGroupSessionKey(             this.roomId, senderKey, sessionId, sentChainIndex,         );          if (!key) { class MegolmEncryption extends EncryptionAlgorithm {             const deviceId = deviceInfo.deviceId;              session.markSharedWithDevice(                 userId, deviceId, key.chain_index,             );         } 

Fixed version (after fix):
 interface IPayload extends Partial<IMessage> { } /* eslint-enable camelcase */  interface SharedWithData {     // The identity key of the device we shared with     deviceKey: string;     // The message index of the ratchet we shared with that device     messageIndex: number; }  /**  * @private  * @constructor interface IPayload extends Partial<IMessage> {  *  * @property {object} sharedWithDevices  *    devices with which we have shared the session key  *        userId -> {deviceId -> SharedWithData}  */ class OutboundSessionInfo {     public useCount = 0;     public creationTime: number;     public sharedWithDevices: Record<string, Record<string, SharedWithData>> = {};     public blockedDevicesNotified: Record<string, Record<string, boolean>> = {};      constructor(public readonly sessionId: string, public readonly sharedHistory = false) { class OutboundSessionInfo {         return false;     }      public markSharedWithDevice(userId: string, deviceId: string, deviceKey: string, chainIndex: number): void {         if (!this.sharedWithDevices[userId]) {             this.sharedWithDevices[userId] = {};         }         this.sharedWithDevices[userId][deviceId] = { deviceKey, messageIndex: chainIndex };     }      public markNotifiedBlockedDevice(userId: string, deviceId: string): void { class MegolmEncryption extends EncryptionAlgorithm {         payload: IPayload,     ): Promise<void> {         const contentMap = {};         const deviceInfoByDeviceId = new Map<string, DeviceInfo>();          const promises = [];         for (let i = 0; i < userDeviceMap.length; i++) { class MegolmEncryption extends EncryptionAlgorithm {             const userId = val.userId;             const deviceInfo = val.deviceInfo;             const deviceId = deviceInfo.deviceId;             deviceInfoByDeviceId.set(deviceId, deviceInfo);              if (!contentMap[userId]) {                 contentMap[userId] = {}; class MegolmEncryption extends EncryptionAlgorithm {                 for (const userId of Object.keys(contentMap)) {                     for (const deviceId of Object.keys(contentMap[userId])) {                         session.markSharedWithDevice(                             userId,                             deviceId,                             deviceInfoByDeviceId.get(deviceId).getIdentityKey(),                             chainIndex,                         );                     }                 } class MegolmEncryption extends EncryptionAlgorithm {             logger.debug(`megolm session ${sessionId} never shared with user ${userId}`);             return;         }         const sessionSharedData = obSessionInfo.sharedWithDevices[userId][device.deviceId];         if (sessionSharedData === undefined) {             logger.debug(                 "megolm session ID " + sessionId + " never shared with device " +                 userId + ":" + device.deviceId,             );             return;         }          if (sessionSharedData.deviceKey !== device.getIdentityKey()) {             logger.warn(                 `Session has been shared with device ${device.deviceId} but with identity ` +                 `key ${sessionSharedData.deviceKey}. Key is now ${device.getIdentityKey()}!`,             );             return;         }          // get the key from the inbound session: the outbound one will already         // have been ratcheted to the next chain index.         const key = await this.olmDevice.getInboundGroupSessionKey(             this.roomId, senderKey, sessionId, sessionSharedData.messageIndex,         );          if (!key) { class MegolmEncryption extends EncryptionAlgorithm {             const deviceId = deviceInfo.deviceId;              session.markSharedWithDevice(                 userId, deviceId, deviceInfo.getIdentityKey(), key.chain_index,             );         } 

Patch:
patch: @@ -101,6 +101,13 @@ interface IPayload extends Partial<IMessage> {
 }
 /* eslint-enable camelcase */
 
+interface SharedWithData {
+    // The identity key of the device we shared with
+    deviceKey: string;
+    // The message index of the ratchet we shared with that device
+    messageIndex: number;
+}
+
 /**
  * @private
  * @constructor
@@ -115,12 +122,12 @@ interface IPayload extends Partial<IMessage> {
  *
  * @property {object} sharedWithDevices
  *    devices with which we have shared the session key
- *        userId -> {deviceId -> msgindex}
+ *        userId -> {deviceId -> SharedWithData}
  */
 class OutboundSessionInfo {
     public useCount = 0;
     public creationTime: number;
-    public sharedWithDevices: Record<string, Record<string, number>> = {};
+    public sharedWithDevices: Record<string, Record<string, SharedWithData>> = {};
     public blockedDevicesNotified: Record<string, Record<string, boolean>> = {};
 
     constructor(public readonly sessionId: string, public readonly sharedHistory = false) {
@@ -150,11 +157,11 @@ class OutboundSessionInfo {
         return false;
     }
 
-    public markSharedWithDevice(userId: string, deviceId: string, chainIndex: number): void {
+    public markSharedWithDevice(userId: string, deviceId: string, deviceKey: string, chainIndex: number): void {
         if (!this.sharedWithDevices[userId]) {
             this.sharedWithDevices[userId] = {};
         }
-        this.sharedWithDevices[userId][deviceId] = chainIndex;
+        this.sharedWithDevices[userId][deviceId] = { deviceKey, messageIndex: chainIndex };
     }
 
     public markNotifiedBlockedDevice(userId: string, deviceId: string): void {
@@ -572,6 +579,7 @@ class MegolmEncryption extends EncryptionAlgorithm {
         payload: IPayload,
     ): Promise<void> {
         const contentMap = {};
+        const deviceInfoByDeviceId = new Map<string, DeviceInfo>();
 
         const promises = [];
         for (let i = 0; i < userDeviceMap.length; i++) {
@@ -584,6 +592,7 @@ class MegolmEncryption extends EncryptionAlgorithm {
             const userId = val.userId;
             const deviceInfo = val.deviceInfo;
             const deviceId = deviceInfo.deviceId;
+            deviceInfoByDeviceId.set(deviceId, deviceInfo);
 
             if (!contentMap[userId]) {
                 contentMap[userId] = {};
@@ -636,7 +645,10 @@ class MegolmEncryption extends EncryptionAlgorithm {
                 for (const userId of Object.keys(contentMap)) {
                     for (const deviceId of Object.keys(contentMap[userId])) {
                         session.markSharedWithDevice(
-                            userId, deviceId, chainIndex,
+                            userId,
+                            deviceId,
+                            deviceInfoByDeviceId.get(deviceId).getIdentityKey(),
+                            chainIndex,
                         );
                     }
                 }
@@ -719,19 +731,27 @@ class MegolmEncryption extends EncryptionAlgorithm {
             logger.debug(`megolm session ${sessionId} never shared with user ${userId}`);
             return;
         }
-        const sentChainIndex = obSessionInfo.sharedWithDevices[userId][device.deviceId];
-        if (sentChainIndex === undefined) {
+        const sessionSharedData = obSessionInfo.sharedWithDevices[userId][device.deviceId];
+        if (sessionSharedData === undefined) {
             logger.debug(
                 "megolm session ID " + sessionId + " never shared with device " +
                 userId + ":" + device.deviceId,
             );
             return;
         }
 
+        if (sessionSharedData.deviceKey !== device.getIdentityKey()) {
+            logger.warn(
+                `Session has been shared with device ${device.deviceId} but with identity ` +
+                `key ${sessionSharedData.deviceKey}. Key is now ${device.getIdentityKey()}!`,
+            );
+            return;
+        }
+
         // get the key from the inbound session: the outbound one will already
         // have been ratcheted to the next chain index.
         const key = await this.olmDevice.getInboundGroupSessionKey(
-            this.roomId, senderKey, sessionId, sentChainIndex,
+            this.roomId, senderKey, sessionId, sessionSharedData.messageIndex,
         );
 
         if (!key) {
@@ -882,7 +902,7 @@ class MegolmEncryption extends EncryptionAlgorithm {
             const deviceId = deviceInfo.deviceId;
 
             session.markSharedWithDevice(
-                userId, deviceId, key.chain_index,
+                userId, deviceId, deviceInfo.getIdentityKey(), key.chain_index,
             );
         }
path: src/crypto/algorithms/megolm.ts