From 4d6f9da5780086f8b6c52beb24ecc8e5ef92f076 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 17 Aug 2016 16:21:37 +0100 Subject: [PATCH] Megolm: don't dereference nullable object It is possible for `room` to be null when passed to MegolmEncryption.encryptMessage; we need to avoid dereferencing it. Instead, make sure that the EncryptionAlgorithm knows about the roomId it is targeting, and use that. Replace the increasingly-long argument list on the EncryptionAlgorithm constructor with a params list, and update DecryptionAlgorithm to match. --- lib/crypto-algorithms/base.js | 30 ++++++++++++++-------------- lib/crypto-algorithms/megolm.js | 35 ++++++++++++++------------------- lib/crypto-algorithms/olm.js | 23 +++++++++------------- lib/crypto.js | 17 ++++++++++++---- 4 files changed, 52 insertions(+), 53 deletions(-) diff --git a/lib/crypto-algorithms/base.js b/lib/crypto-algorithms/base.js index 95b335ff5..f50b94a96 100644 --- a/lib/crypto-algorithms/base.js +++ b/lib/crypto-algorithms/base.js @@ -44,14 +44,17 @@ module.exports.DECRYPTION_CLASSES = {}; * * @constructor * - * @param {string} deviceId The identifier for this device. - * @param {module:crypto} crypto crypto core - * @param {module:OlmDevice} olmDevice olm.js wrapper + * @param {object} params parameters + * @param {string} params.deviceId The identifier for this device. + * @param {module:crypto} params.crypto crypto core + * @param {module:OlmDevice} params.olmDevice olm.js wrapper + * @param {string} params.roomId The ID of the room we will be sending to */ -module.exports.EncryptionAlgorithm = function(deviceId, crypto, olmDevice) { - this._deviceId = deviceId; - this._crypto = crypto; - this._olmDevice = olmDevice; +module.exports.EncryptionAlgorithm = function(params) { + this._deviceId = params.deviceId; + this._crypto = params.crypto; + this._olmDevice = params.olmDevice; + this._roomId = params.roomId; }; /** @@ -70,7 +73,7 @@ module.exports.EncryptionAlgorithm = function(deviceId, crypto, olmDevice) { * @method module:crypto-algorithms/base.EncryptionAlgorithm#encryptMessage * @abstract * - * @param {module:models/room} room + * @param {module:models/room?} room * @param {string} eventType * @param {object} plaintext event content * @@ -82,14 +85,11 @@ module.exports.EncryptionAlgorithm = function(deviceId, crypto, olmDevice) { * base type for decryption implementations * * @constructor - * @param {string} deviceId The identifier for this device. - * @param {module:crypto} crypto crypto core - * @param {module:OlmDevice} olmDevice olm.js wrapper + * @param {object} params parameters + * @param {module:OlmDevice} params.olmDevice olm.js wrapper */ -module.exports.DecryptionAlgorithm = function(deviceId, crypto, olmDevice) { - this._deviceId = deviceId; - this._crypto = crypto; - this._olmDevice = olmDevice; +module.exports.DecryptionAlgorithm = function(params) { + this._olmDevice = params.olmDevice; }; /** diff --git a/lib/crypto-algorithms/megolm.js b/lib/crypto-algorithms/megolm.js index 840d3025d..16993fca6 100644 --- a/lib/crypto-algorithms/megolm.js +++ b/lib/crypto-algorithms/megolm.js @@ -18,7 +18,7 @@ limitations under the License. /** * Defines m.olm encryption/decryption * - * @module crypto-algorithms/olm + * @module crypto-algorithms/megolm */ var q = require("q"); @@ -34,13 +34,11 @@ var MEGOLM_ALGORITHM = "m.megolm.v1.aes-sha2"; * @constructor * @extends {module:crypto-algorithms/base.EncryptionAlgorithm} * - * @param {string} deviceId The identifier for this device. - * @param {module:crypto} crypto crypto core - * @param {module:OlmDevice} olmDevice olm.js wrapper - * + * @param {object} params parameters, as per + * {@link module:crypto-algorithms/base.EncryptionAlgorithm} */ -function MegolmEncryption(deviceId, crypto, olmDevice) { - base.EncryptionAlgorithm.call(this, deviceId, crypto, olmDevice); +function MegolmEncryption(params) { + base.EncryptionAlgorithm.call(this, params); this._outboundSessionId = null; } utils.inherits(MegolmEncryption, base.EncryptionAlgorithm); @@ -58,9 +56,8 @@ MegolmEncryption.prototype.initRoomEncryption = function(roomMembers) { /** * @private - * @param {string} roomId */ -MegolmEncryption.prototype._ensureOutboundSession = function(roomId) { +MegolmEncryption.prototype._ensureOutboundSession = function() { if (this._outboundSessionId) { return; } @@ -75,14 +72,14 @@ MegolmEncryption.prototype._ensureOutboundSession = function(roomId) { 'Created outbound session. Add with window.mxMatrixClientPeg.' + 'matrixClient._crypto._olmDevice.addInboundGroupSession("' + [ - roomId, this._olmDevice.deviceCurve25519Key, session_id, + this._roomId, this._olmDevice.deviceCurve25519Key, session_id, key.key, key.chain_index ].join('", "') + '")' ); this._olmDevice.addInboundGroupSession( - roomId, this._olmDevice.deviceCurve25519Key, session_id, + this._roomId, this._olmDevice.deviceCurve25519Key, session_id, key.key, key.chain_index ); }; @@ -90,17 +87,17 @@ MegolmEncryption.prototype._ensureOutboundSession = function(roomId) { /** * @inheritdoc * - * @param {module:models/room} room + * @param {module:models/room?} room * @param {string} eventType * @param {object} plaintext event content * * @return {object} new event body */ MegolmEncryption.prototype.encryptMessage = function(room, eventType, content) { - this._ensureOutboundSession(room.roomId); + this._ensureOutboundSession(); var payloadJson = { - room_id: room.roomId, + room_id: this._roomId, type: eventType, content: content }; @@ -126,13 +123,11 @@ MegolmEncryption.prototype.encryptMessage = function(room, eventType, content) { * @constructor * @extends {module:crypto-algorithms/base.DecryptionAlgorithm} * - * @param {string} deviceId The identifier for this device. - * @param {module:crypto} crypto crypto core - * @param {module:OlmDevice} olmDevice olm.js wrapper - * + * @param {object} params parameters, as per + * {@link module:crypto-algorithms/base.DecryptionAlgorithm} */ -function MegolmDecryption(deviceId, crypto, olmDevice) { - base.DecryptionAlgorithm.call(this, deviceId, crypto, olmDevice); +function MegolmDecryption(params) { + base.DecryptionAlgorithm.call(this, params); } utils.inherits(MegolmDecryption, base.DecryptionAlgorithm); diff --git a/lib/crypto-algorithms/olm.js b/lib/crypto-algorithms/olm.js index b8229c9b3..e17d3968b 100644 --- a/lib/crypto-algorithms/olm.js +++ b/lib/crypto-algorithms/olm.js @@ -33,13 +33,11 @@ var OLM_ALGORITHM = "m.olm.v1.curve25519-aes-sha2"; * @constructor * @extends {module:crypto-algorithms/base.EncryptionAlgorithm} * - * @param {string} deviceId The identifier for this device. - * @param {module:crypto} crypto crypto core - * @param {module:OlmDevice} olmDevice olm.js wrapper - * + * @param {object} params parameters, as per + * {@link module:crypto-algorithms/base.EncryptionAlgorithm} */ -function OlmEncryption(deviceId, crypto, olmDevice) { - base.EncryptionAlgorithm.call(this, deviceId, crypto, olmDevice); +function OlmEncryption(params) { + base.EncryptionAlgorithm.call(this, params); } utils.inherits(OlmEncryption, base.EncryptionAlgorithm); @@ -58,7 +56,7 @@ OlmEncryption.prototype.initRoomEncryption = function(roomMembers) { /** * @inheritdoc * - * @param {module:models/room} room + * @param {module:models/room?} room * @param {string} eventType * @param {object} plaintext event content * @@ -138,14 +136,11 @@ OlmEncryption.prototype.encryptMessage = function(room, eventType, content) { * * @constructor * @extends {module:crypto-algorithms/base.DecryptionAlgorithm} - * - * @param {string} deviceId The identifier for this device. - * @param {module:crypto} crypto crypto core - * @param {module:OlmDevice} olmDevice olm.js wrapper - * + * @param {object} params parameters, as per + * {@link module:crypto-algorithms/base.DecryptionAlgorithm} */ -function OlmDecryption(deviceId, crypto, olmDevice) { - base.DecryptionAlgorithm.call(this, deviceId, crypto, olmDevice); +function OlmDecryption(params) { + base.DecryptionAlgorithm.call(this, params); } utils.inherits(OlmDecryption, base.DecryptionAlgorithm); diff --git a/lib/crypto.js b/lib/crypto.js index 80923bb8f..fcb00b9a8 100644 --- a/lib/crypto.js +++ b/lib/crypto.js @@ -625,7 +625,12 @@ Crypto.prototype.setRoomEncryption = function(roomId, config, roomMembers) { }; this._sessionStore.storeEndToEndRoom(roomId, config); - var alg = new AlgClass(this._deviceId, this, this._olmDevice); + var alg = new AlgClass({ + deviceId: this._deviceId, + crypto: this, + olmDevice: this._olmDevice, + roomId: roomId, + }); this._roomAlgorithms[roomId] = alg; return alg.initRoomEncryption(roomMembers); }; @@ -719,7 +724,9 @@ Crypto.prototype.isRoomEncrypted = function(roomId) { * Encrypt an event according to the configuration of the room, if necessary. * * @param {module:models/event.MatrixEvent} event event to be sent - * @param {module:models/room} room destination room + * + * @param {module:models/room?} room destination room. Null if the destination + * is not a room we have seen over the sync pipe. */ Crypto.prototype.encryptEventIfNeeded = function(event, room) { if (event.isEncrypted()) { @@ -752,7 +759,7 @@ Crypto.prototype.encryptEventIfNeeded = function(event, room) { } var encryptedContent = alg.encryptMessage( - room, event.getType(), event.getContent() + room, event.getType(), event.getContent() ); event.makeEncrypted("m.room.encrypted", encryptedContent); }; @@ -772,7 +779,9 @@ Crypto.prototype.decryptEvent = function(event) { if (!AlgClass) { throw new algorithms.DecryptionError("Unable to decrypt " + content.algorithm); } - var alg = new AlgClass(this._deviceId, this, this._olmDevice); + var alg = new AlgClass({ + olmDevice: this._olmDevice, + }); return alg.decryptEvent(event); };