From e2341b1a06e2f82d6871e6edb8527c48bae0946f Mon Sep 17 00:00:00 2001 From: Maarten Zuidhoorn Date: Fri, 14 Mar 2025 14:00:51 +0100 Subject: [PATCH 1/3] Add mutex to `getSnapState` to prevent concurrent decryption --- .../src/snaps/SnapController.ts | 48 +++++++++++-------- 1 file changed, 28 insertions(+), 20 deletions(-) diff --git a/packages/snaps-controllers/src/snaps/SnapController.ts b/packages/snaps-controllers/src/snaps/SnapController.ts index 5c8b635e87..6da9116b9f 100644 --- a/packages/snaps-controllers/src/snaps/SnapController.ts +++ b/packages/snaps-controllers/src/snaps/SnapController.ts @@ -286,6 +286,11 @@ export type SnapRuntimeData = { * A mutex to prevent concurrent state updates. */ stateMutex: Mutex; + + /** + * A mutex to prevent concurrent state decryption. + */ + getStateMutex: Mutex; }; export type SnapError = { @@ -2046,33 +2051,35 @@ export class SnapController extends BaseController< */ async getSnapState(snapId: SnapId, encrypted: boolean): Promise { const runtime = this.#getRuntimeExpect(snapId); - const cachedState = encrypted ? runtime.state : runtime.unencryptedState; + return await runtime.getStateMutex.runExclusive(() => { + const cachedState = encrypted ? runtime.state : runtime.unencryptedState; - if (cachedState !== undefined) { - return cachedState; - } + if (cachedState !== undefined) { + return cachedState; + } - const state = encrypted - ? this.state.snapStates[snapId] - : this.state.unencryptedSnapStates[snapId]; + const state = encrypted + ? this.state.snapStates[snapId] + : this.state.unencryptedSnapStates[snapId]; - if (state === null || state === undefined) { - return null; - } + if (state === null || state === undefined) { + return null; + } - if (!encrypted) { - // For performance reasons, we do not validate that the state is JSON, - // since we control serialization. - const json = JSON.parse(state); - runtime.unencryptedState = json; + if (!encrypted) { + // For performance reasons, we do not validate that the state is JSON, + // since we control serialization. + const json = JSON.parse(state); + runtime.unencryptedState = json; - return json; - } + return json; + } - const decrypted = await this.#decryptSnapState(snapId, state); - runtime.state = decrypted; + const decrypted = await this.#decryptSnapState(snapId, state); + runtime.state = decrypted; - return decrypted; + return decrypted; + }); } /** @@ -3968,6 +3975,7 @@ export class SnapController extends BaseController< interpreter, stopping: false, stateMutex: new Mutex(), + getStateMutex: new Mutex(), }); } From d85a70bb4466f278885dd2946d0405dcae14a6ac Mon Sep 17 00:00:00 2001 From: Maarten Zuidhoorn Date: Fri, 14 Mar 2025 14:09:12 +0100 Subject: [PATCH 2/3] Add missing async --- packages/snaps-controllers/src/snaps/SnapController.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/snaps-controllers/src/snaps/SnapController.ts b/packages/snaps-controllers/src/snaps/SnapController.ts index 6da9116b9f..90cb50cc76 100644 --- a/packages/snaps-controllers/src/snaps/SnapController.ts +++ b/packages/snaps-controllers/src/snaps/SnapController.ts @@ -2051,7 +2051,7 @@ export class SnapController extends BaseController< */ async getSnapState(snapId: SnapId, encrypted: boolean): Promise { const runtime = this.#getRuntimeExpect(snapId); - return await runtime.getStateMutex.runExclusive(() => { + return await runtime.getStateMutex.runExclusive(async () => { const cachedState = encrypted ? runtime.state : runtime.unencryptedState; if (cachedState !== undefined) { @@ -2076,6 +2076,7 @@ export class SnapController extends BaseController< } const decrypted = await this.#decryptSnapState(snapId, state); + // eslint-disable-next-line require-atomic-updates runtime.state = decrypted; return decrypted; From 64549e3f5f24ff71b73c98bd9a784c8859a3f869 Mon Sep 17 00:00:00 2001 From: Maarten Zuidhoorn Date: Fri, 14 Mar 2025 14:25:45 +0100 Subject: [PATCH 3/3] Update coverage --- packages/snaps-controllers/coverage.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/snaps-controllers/coverage.json b/packages/snaps-controllers/coverage.json index cedb13e5ab..6df3e67f1f 100644 --- a/packages/snaps-controllers/coverage.json +++ b/packages/snaps-controllers/coverage.json @@ -1,6 +1,6 @@ { "branches": 93.34, - "functions": 97.36, + "functions": 97.37, "lines": 98.33, "statements": 98.07 }