From 5c0920da425a13e5b7d72461baab383266d61a0a Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 12 May 2020 11:05:30 +0100 Subject: [PATCH] Avoid soft crash if unknown device in verification Rageshakes from the wild indicate that device was null here which implies that we somehow did not know about the device when verifiying it? Log and null-check to avoid a soft crash. --- .../views/right_panel/EncryptionPanel.js | 5 +-- .../views/right_panel/VerificationPanel.js | 32 ++++++++++++++----- .../views/verification/VerificationShowSas.js | 7 ++-- 3 files changed, 30 insertions(+), 14 deletions(-) diff --git a/src/components/views/right_panel/EncryptionPanel.js b/src/components/views/right_panel/EncryptionPanel.js index bc580c767b..e9f94729fa 100644 --- a/src/components/views/right_panel/EncryptionPanel.js +++ b/src/components/views/right_panel/EncryptionPanel.js @@ -45,9 +45,6 @@ const EncryptionPanel = (props) => { } }, [verificationRequest]); - const deviceId = request && request.channel.deviceId; - const device = MatrixClientPeg.get().getStoredDevice(MatrixClientPeg.get().getUserId(), deviceId); - useEffect(() => { async function awaitPromise() { setRequesting(true); @@ -143,7 +140,7 @@ const EncryptionPanel = (props) => { key={request.channel.transactionId} inDialog={layout === "dialog"} phase={phase} - device={device} /> + /> ); } }; diff --git a/src/components/views/right_panel/VerificationPanel.js b/src/components/views/right_panel/VerificationPanel.js index 67efd29d27..ff3700e143 100644 --- a/src/components/views/right_panel/VerificationPanel.js +++ b/src/components/views/right_panel/VerificationPanel.js @@ -17,6 +17,7 @@ limitations under the License. import React from "react"; import PropTypes from "prop-types"; +import {MatrixClientPeg} from "../../../MatrixClientPeg"; import * as sdk from '../../../index'; import {verificationMethods} from 'matrix-js-sdk/src/crypto'; import {SCAN_QR_CODE_METHOD} from "matrix-js-sdk/src/crypto/verification/QRCode"; @@ -161,6 +162,11 @@ export default class VerificationPanel extends React.PureComponent { this.state.reciprocateQREvent.cancel(); }; + _getDevice() { + const deviceId = this.props.request && this.props.request.channel.deviceId; + return MatrixClientPeg.get().getStoredDevice(MatrixClientPeg.get().getUserId(), deviceId); + } + renderQRReciprocatePhase() { const {member, request} = this.props; let Button; @@ -217,16 +223,26 @@ export default class VerificationPanel extends React.PureComponent { } } - const AccessibleButton = sdk.getComponent('elements.AccessibleButton'); - const description = request.isSelfVerification ? - _t("You've successfully verified %(deviceName)s (%(deviceId)s)!", { - deviceName: this.props.device.getDisplayName(), - deviceId: this.props.device.deviceId, - }): - _t("You've successfully verified %(displayName)s!", { + let description; + if (request.isSelfVerification) { + const device = this._getDevice(); + if (!device) { + // This shouldn't happen, although rageshake would indicate that it is: log a warn + // and leave the message slightly broken (avoid adding a translatable string for a + // case that shouldn't be happening). + console.warn("Verified device we don't know about: " + this.props.request.channel.deviceId); + } + description = _t("You've successfully verified %(deviceName)s (%(deviceId)s)!", { + deviceName: device ? device.getDisplayName() : '', + deviceId: this.props.request.channel.deviceId, + }); + } else { + description = _t("You've successfully verified %(displayName)s!", { displayName: member.displayName || member.name || member.userId, }); + } + const AccessibleButton = sdk.getComponent('elements.AccessibleButton'); return (

{_t("Verified")}

@@ -297,12 +313,12 @@ export default class VerificationPanel extends React.PureComponent { const emojis = this.state.sasEvent ? : ; return

{_t("Compare emoji")}

diff --git a/src/components/views/verification/VerificationShowSas.js b/src/components/views/verification/VerificationShowSas.js index edf860c4c2..251c8ca04f 100644 --- a/src/components/views/verification/VerificationShowSas.js +++ b/src/components/views/verification/VerificationShowSas.js @@ -30,6 +30,7 @@ export default class VerificationShowSas extends React.Component { static propTypes = { pending: PropTypes.bool, displayName: PropTypes.string, // required if pending is true + device: PropTypes.object, onDone: PropTypes.func.isRequired, onCancel: PropTypes.func.isRequired, sas: PropTypes.object.isRequired, @@ -116,9 +117,11 @@ export default class VerificationShowSas extends React.Component { let text; if (this.state.pending) { if (this.props.isSelf) { + // device shouldn't be null in this situation but it seems like it can be, so show + // slightly broken text rather than soft-crashing (we've already logged in VerificationPanel). text = _t("Waiting for your other session, %(deviceName)s (%(deviceId)s), to verify…", { - deviceName: this.props.device.getDisplayName(), - deviceId: this.props.device.deviceId, + deviceName: this.props.device ? this.props.device.getDisplayName() : '', + deviceId: this.props.device ? this.props.device.deviceId : '', }); } else { const {displayName} = this.props;