From a7496627fcc327601a2973497064ccd8fa32263d Mon Sep 17 00:00:00 2001
From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
Date: Tue, 28 Nov 2023 13:14:53 +0000
Subject: [PATCH] Explicitly free some Rust-side objects (#3911)

* Explicitly `free` stuff returned by `OlmMachine.getIdentity()`

* Explicitly `free` stuff returned by `OlmMachine.getDevice()`

* one more
---
 spec/unit/rust-crypto/rust-crypto.spec.ts |   3 +-
 src/rust-crypto/CrossSigningIdentity.ts   |  11 +-
 src/rust-crypto/rust-crypto.ts            | 175 ++++++++++++++--------
 3 files changed, 120 insertions(+), 69 deletions(-)

diff --git a/spec/unit/rust-crypto/rust-crypto.spec.ts b/spec/unit/rust-crypto/rust-crypto.spec.ts
index 1044c71a0..7685f811d 100644
--- a/spec/unit/rust-crypto/rust-crypto.spec.ts
+++ b/spec/unit/rust-crypto/rust-crypto.spec.ts
@@ -645,6 +645,7 @@ describe("RustCrypto", () => {
 
         it("should call getDevice", async () => {
             olmMachine.getDevice.mockResolvedValue({
+                free: jest.fn(),
                 isCrossSigningTrusted: jest.fn().mockReturnValue(false),
                 isLocallyTrusted: jest.fn().mockReturnValue(false),
                 isCrossSignedByOwner: jest.fn().mockReturnValue(false),
@@ -871,7 +872,7 @@ describe("RustCrypto", () => {
         });
 
         it("returns a verified UserVerificationStatus when the UserIdentity is verified", async () => {
-            olmMachine.getIdentity.mockResolvedValue({ isVerified: jest.fn().mockReturnValue(true) });
+            olmMachine.getIdentity.mockResolvedValue({ free: jest.fn(), isVerified: jest.fn().mockReturnValue(true) });
 
             const userVerificationStatus = await rustCrypto.getUserVerificationStatus(testData.TEST_USER_ID);
             expect(userVerificationStatus.isVerified()).toBeTruthy();
diff --git a/src/rust-crypto/CrossSigningIdentity.ts b/src/rust-crypto/CrossSigningIdentity.ts
index 8598dab61..c0bccec10 100644
--- a/src/rust-crypto/CrossSigningIdentity.ts
+++ b/src/rust-crypto/CrossSigningIdentity.ts
@@ -91,10 +91,13 @@ export class CrossSigningIdentity {
                     this.olmMachine.userId,
                     this.olmMachine.deviceId,
                 );
-
-                // Sign the device with our cross-signing key and upload the signature
-                const request: RustSdkCryptoJs.SignatureUploadRequest = await device.verify();
-                await this.outgoingRequestProcessor.makeOutgoingRequest(request);
+                try {
+                    // Sign the device with our cross-signing key and upload the signature
+                    const request: RustSdkCryptoJs.SignatureUploadRequest = await device.verify();
+                    await this.outgoingRequestProcessor.makeOutgoingRequest(request);
+                } finally {
+                    device.free();
+                }
             } else {
                 logger.log(
                     "bootStrapCrossSigning: Cross-signing private keys not found locally or in secret storage, creating new keys",
diff --git a/src/rust-crypto/rust-crypto.ts b/src/rust-crypto/rust-crypto.ts
index f0c3c8973..dd90a44b6 100644
--- a/src/rust-crypto/rust-crypto.ts
+++ b/src/rust-crypto/rust-crypto.ts
@@ -425,6 +425,7 @@ export class RustCrypto extends TypedEventEmitter<RustCryptoEvents, RustCryptoEv
                 await this.outgoingRequestProcessor.makeOutgoingRequest(request);
             }
             const userIdentity = await this.olmMachine.getIdentity(rustTrackedUser);
+            userIdentity?.free();
             return userIdentity !== undefined;
         } else if (downloadUncached) {
             // Download the cross signing keys and check if the master key is available
@@ -562,7 +563,13 @@ export class RustCrypto extends TypedEventEmitter<RustCryptoEvents, RustCryptoEv
         if (!device) {
             throw new Error(`Unknown device ${userId}|${deviceId}`);
         }
-        await device.setLocalTrust(verified ? RustSdkCryptoJs.LocalTrust.Verified : RustSdkCryptoJs.LocalTrust.Unset);
+        try {
+            await device.setLocalTrust(
+                verified ? RustSdkCryptoJs.LocalTrust.Verified : RustSdkCryptoJs.LocalTrust.Unset,
+            );
+        } finally {
+            device.free();
+        }
     }
 
     /**
@@ -578,13 +585,16 @@ export class RustCrypto extends TypedEventEmitter<RustCryptoEvents, RustCryptoEv
         );
 
         if (!device) return null;
-
-        return new DeviceVerificationStatus({
-            signedByOwner: device.isCrossSignedByOwner(),
-            crossSigningVerified: device.isCrossSigningTrusted(),
-            localVerified: device.isLocallyTrusted(),
-            trustCrossSignedDevices: this._trustCrossSignedDevices,
-        });
+        try {
+            return new DeviceVerificationStatus({
+                signedByOwner: device.isCrossSignedByOwner(),
+                crossSigningVerified: device.isCrossSigningTrusted(),
+                localVerified: device.isLocallyTrusted(),
+                trustCrossSignedDevices: this._trustCrossSignedDevices,
+            });
+        } finally {
+            device.free();
+        }
     }
 
     /**
@@ -596,7 +606,9 @@ export class RustCrypto extends TypedEventEmitter<RustCryptoEvents, RustCryptoEv
         if (userIdentity === undefined) {
             return new UserVerificationStatus(false, false, false);
         }
-        return new UserVerificationStatus(userIdentity.isVerified(), false, false);
+        const verified = userIdentity.isVerified();
+        userIdentity.free();
+        return new UserVerificationStatus(verified, false, false);
     }
 
     /**
@@ -621,42 +633,51 @@ export class RustCrypto extends TypedEventEmitter<RustCryptoEvents, RustCryptoEv
         const userIdentity: RustSdkCryptoJs.OwnUserIdentity | undefined = await this.olmMachine.getIdentity(
             new RustSdkCryptoJs.UserId(this.userId),
         );
-
-        const crossSigningStatus: RustSdkCryptoJs.CrossSigningStatus = await this.olmMachine.crossSigningStatus();
-        const privateKeysOnDevice =
-            crossSigningStatus.hasMaster && crossSigningStatus.hasUserSigning && crossSigningStatus.hasSelfSigning;
-
-        if (!userIdentity || !privateKeysOnDevice) {
-            // The public or private keys are not available on this device
+        if (!userIdentity) {
+            // The public keys are not available on this device
             return null;
         }
 
-        if (!userIdentity.isVerified()) {
-            // We have both public and private keys, but they don't match!
-            return null;
-        }
+        try {
+            const crossSigningStatus: RustSdkCryptoJs.CrossSigningStatus = await this.olmMachine.crossSigningStatus();
 
-        let key: string;
-        switch (type) {
-            case CrossSigningKey.Master:
-                key = userIdentity.masterKey;
-                break;
-            case CrossSigningKey.SelfSigning:
-                key = userIdentity.selfSigningKey;
-                break;
-            case CrossSigningKey.UserSigning:
-                key = userIdentity.userSigningKey;
-                break;
-            default:
-                // Unknown type
+            const privateKeysOnDevice =
+                crossSigningStatus.hasMaster && crossSigningStatus.hasUserSigning && crossSigningStatus.hasSelfSigning;
+
+            if (!privateKeysOnDevice) {
+                // The private keys are not available on this device
                 return null;
-        }
+            }
 
-        const parsedKey: CrossSigningKeyInfo = JSON.parse(key);
-        // `keys` is an object with { [`ed25519:${pubKey}`]: pubKey }
-        // We assume only a single key, and we want the bare form without type
-        // prefix, so we select the values.
-        return Object.values(parsedKey.keys)[0];
+            if (!userIdentity.isVerified()) {
+                // We have both public and private keys, but they don't match!
+                return null;
+            }
+
+            let key: string;
+            switch (type) {
+                case CrossSigningKey.Master:
+                    key = userIdentity.masterKey;
+                    break;
+                case CrossSigningKey.SelfSigning:
+                    key = userIdentity.selfSigningKey;
+                    break;
+                case CrossSigningKey.UserSigning:
+                    key = userIdentity.userSigningKey;
+                    break;
+                default:
+                    // Unknown type
+                    return null;
+            }
+
+            const parsedKey: CrossSigningKeyInfo = JSON.parse(key);
+            // `keys` is an object with { [`ed25519:${pubKey}`]: pubKey }
+            // We assume only a single key, and we want the bare form without type
+            // prefix, so we select the values.
+            return Object.values(parsedKey.keys)[0];
+        } finally {
+            userIdentity.free();
+        }
     }
 
     /**
@@ -800,6 +821,8 @@ export class RustCrypto extends TypedEventEmitter<RustCryptoEvents, RustCryptoEv
             Boolean(userIdentity?.masterKey) &&
             Boolean(userIdentity?.selfSigningKey) &&
             Boolean(userIdentity?.userSigningKey);
+        userIdentity?.free();
+
         const privateKeysInSecretStorage = await secretStorageContainsCrossSigningKeys(this.secretStorage);
         const crossSigningStatus: RustSdkCryptoJs.CrossSigningStatus | null =
             await this.getOlmMachineOrThrow().crossSigningStatus();
@@ -917,23 +940,31 @@ export class RustCrypto extends TypedEventEmitter<RustCryptoEvents, RustCryptoEv
 
         if (!userIdentity) throw new Error(`unknown userId ${userId}`);
 
-        // Transform the verification methods into rust objects
-        const methods = this._supportedVerificationMethods.map((method) =>
-            verificationMethodIdentifierToMethod(method),
-        );
-        // Get the request content to send to the DM room
-        const verificationEventContent: string = await userIdentity.verificationRequestContent(methods);
+        try {
+            // Transform the verification methods into rust objects
+            const methods = this._supportedVerificationMethods.map((method) =>
+                verificationMethodIdentifierToMethod(method),
+            );
+            // Get the request content to send to the DM room
+            const verificationEventContent: string = await userIdentity.verificationRequestContent(methods);
 
-        // Send the request content to send to the DM room
-        const eventId = await this.sendVerificationRequestContent(roomId, verificationEventContent);
+            // Send the request content to send to the DM room
+            const eventId = await this.sendVerificationRequestContent(roomId, verificationEventContent);
 
-        // Get a verification request
-        const request: RustSdkCryptoJs.VerificationRequest = await userIdentity.requestVerification(
-            new RustSdkCryptoJs.RoomId(roomId),
-            new RustSdkCryptoJs.EventId(eventId),
-            methods,
-        );
-        return new RustVerificationRequest(request, this.outgoingRequestProcessor, this._supportedVerificationMethods);
+            // Get a verification request
+            const request: RustSdkCryptoJs.VerificationRequest = await userIdentity.requestVerification(
+                new RustSdkCryptoJs.RoomId(roomId),
+                new RustSdkCryptoJs.EventId(eventId),
+                methods,
+            );
+            return new RustVerificationRequest(
+                request,
+                this.outgoingRequestProcessor,
+                this._supportedVerificationMethods,
+            );
+        } finally {
+            userIdentity.free();
+        }
     }
 
     /**
@@ -995,12 +1026,20 @@ export class RustCrypto extends TypedEventEmitter<RustCryptoEvents, RustCryptoEv
             throw new Error("cannot request verification for this device when there is no existing cross-signing key");
         }
 
-        const [request, outgoingRequest]: [RustSdkCryptoJs.VerificationRequest, RustSdkCryptoJs.ToDeviceRequest] =
-            await userIdentity.requestVerification(
-                this._supportedVerificationMethods.map(verificationMethodIdentifierToMethod),
+        try {
+            const [request, outgoingRequest]: [RustSdkCryptoJs.VerificationRequest, RustSdkCryptoJs.ToDeviceRequest] =
+                await userIdentity.requestVerification(
+                    this._supportedVerificationMethods.map(verificationMethodIdentifierToMethod),
+                );
+            await this.outgoingRequestProcessor.makeOutgoingRequest(outgoingRequest);
+            return new RustVerificationRequest(
+                request,
+                this.outgoingRequestProcessor,
+                this._supportedVerificationMethods,
             );
-        await this.outgoingRequestProcessor.makeOutgoingRequest(outgoingRequest);
-        return new RustVerificationRequest(request, this.outgoingRequestProcessor, this._supportedVerificationMethods);
+        } finally {
+            userIdentity.free();
+        }
     }
 
     /**
@@ -1025,12 +1064,20 @@ export class RustCrypto extends TypedEventEmitter<RustCryptoEvents, RustCryptoEv
             throw new Error("Not a known device");
         }
 
-        const [request, outgoingRequest]: [RustSdkCryptoJs.VerificationRequest, RustSdkCryptoJs.ToDeviceRequest] =
-            await device.requestVerification(
-                this._supportedVerificationMethods.map(verificationMethodIdentifierToMethod),
+        try {
+            const [request, outgoingRequest]: [RustSdkCryptoJs.VerificationRequest, RustSdkCryptoJs.ToDeviceRequest] =
+                await device.requestVerification(
+                    this._supportedVerificationMethods.map(verificationMethodIdentifierToMethod),
+                );
+            await this.outgoingRequestProcessor.makeOutgoingRequest(outgoingRequest);
+            return new RustVerificationRequest(
+                request,
+                this.outgoingRequestProcessor,
+                this._supportedVerificationMethods,
             );
-        await this.outgoingRequestProcessor.makeOutgoingRequest(outgoingRequest);
-        return new RustVerificationRequest(request, this.outgoingRequestProcessor, this._supportedVerificationMethods);
+        } finally {
+            device.free();
+        }
     }
 
     /**