MetaMask / MetaMask/metamask-extension

[Bug]: Messenger subscription leak in PPOM security validation waitForTransactionMetadata/waitForSignatureRequest never rejects or times out

Open
#45,129 1 comment 0 reactions 0 assignees View on GitHub
cs-triaged external-contributor INVALID-ISSUE-TEMPLATE regression-prod-* Sev2-normal team-application-security team-confirmations type-bug
Dominant language
TypeScript
Stars
13.2k
Forks
5.6k
Avg merge
2d 5h
Merged PRs (30d)
451

Description

**Describe the bug**

`waitForTransactionMetadata` and `waitForSignatureRequest` in `app/scripts/lib/ppom/ppom-util.ts` return Promises that only resolve and never reject, with no timeout. Inside `validateRequestWithPPOM`, `updateSecurityAlertResponse` is called **twice** per request once with `LOADING_SECURITY_ALERT_RESPONSE` (line 77) and once with the final PPOM result (line 100/104). Each call invokes `waitForTransactionMetadata` or `waitForSignatureRequest`, which subscribe to a messenger event.

**Race condition that causes permanent leak:**
1. User submits `eth_sendTransaction` or a signing method from a dApp
2. First `waitForTransactionMetadata` call subscribes to `TransactionController:unapprovedTransactionAdded`, finds the tx → resolves ✓
3. PPOM makes a network request to the security alerts API (2–10 seconds)
4. **User rejects/cancels the confirmation dialog while PPOM is running**
5. Second `waitForTransactionMetadata` call: `state.transactions.find(filter)` returns `undefined` (tx was removed on rejection)
6. Subscribes to `TransactionController:unapprovedTransactionAdded`
7. **This event never fires** the tx was already rejected
8. **Subscription leaks permanently** for the remainder of the service worker session

**`waitForSignatureRequest` is worse:** it subscribes to `SignatureController:stateChange`, which fires on **every** signature state change. Each leaked callback runs on all future signature events. With N rejected signatures during a session, N callbacks accumulate and execute on every subsequent signature event a performance death spiral.

**Expected behavior**

Both functions should have a timeout (e.g. 60s) and reject with cleanup if the expected event never arrives. The messenger subscription must be unsubscribed in all exit paths.

**Steps to reproduce**

1. Connect a dApp to MetaMask
2. Trigger eth_sendTransaction or eth_signTypedData from the dApp
3. While the PPOM security check is running (visible as loading state in confirmation UI), reject the confirmation
4. Repeat multiple times
5. Observe growing messenger subscription count each rejected confirmation leaks one subscription to TransactionController:unapprovedTransactionAdded or SignatureController:stateChange

**Error messages or log output**

```shell
No thrown error silent resource leak.

Root cause:
File: app/scripts/lib/ppom/ppom-util.ts

waitForTransactionMetadata (line 374): returns new Promise((resolve) => {...})
— no reject path, no timeout, subscription never cleaned up on abandon

waitForSignatureRequest (line 418): returns new Promise((resolve) => {...})
— subscribes to SignatureController:stateChange (fires on ALL state changes)
— leaked callbacks accumulate and degrade performance

validateRequestWithPPOM calls updateSecurityAlertResponse TWICE:
line 77: await updateSecurityResponse(method, id, LOADING_RESPONSE) // first wait
line 100: await updateSecurityResponse(method, id, ppomResponse) // second wait — leaks here

Fix: add timeout + reject + cleanup in both functions:
return new Promise((resolve, reject) => {
const timeoutId = setTimeout(() => {
messenger.unsubscribe(EVENT, callback);
reject(new Error(`Timed out waiting for securityAlertId: ${securityAlertId}`));
}, 60_000);
const callback = (...) => {
clearTimeout(timeoutId);
messenger.unsubscribe(EVENT, callback);
resolve(...);
};
messenger.subscribe(EVENT, callback);
});
```

**Where was this bug found?**

Live version (from official store)

**Version**

12.x (main branch)

**Build type**

None

**Browser**

Chrome

**Operating system**

Linux, MacOS, Windows

**Additional context**

Affects all users who reject/cancel transactions or signature requests while PPOM security validation is in progress. The leak grows with each cancelled confirmation during a service worker session. For waitForSignatureRequest specifically, each leaked callback fires synchronously on every future SignatureController:stateChange event, degrading performance cumulatively.

Contributor guide

Open the contributing guide

Research direction

Start in app/scripts/lib/ppom/ppom-util.ts, reading waitForTransactionMetadata, waitForSignatureRequest, and validateRequestWithPPOM together with their messenger event subscriptions. Trace the cancellation path and verify that both waits reject or time out and unsubscribe on every exit path, including when the expected event never arrives.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
security
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.