spend-permission: requestSpendPermission returns inconsistent SpendPermission when wallet substitutes account via mutableData
- Dominant language
- TypeScript
- Stars
- 172
- Forks
- 207
- Avg merge
- 49m
- Merged PRs (30d)
- 1
Description
## Summary
`requestSpendPermission` declares that the wallet may mutate `message.account` via `mutableData`, but then constructs the returned `SpendPermission` object using the **original** (pre-substitution) typed data for the `permission` field while using the **wallet-returned** (post-substitution) signed data only for `permissionHash`. This produces an internally inconsistent object.
## Affected file
`packages/account-sdk/src/interface/public-utilities/spend-permission/methods/requestSpendPermission.ts` — HEAD `24ab30c` (account 2.5.6)
## Code path
When `capabilities` is provided (lines 82-128), the function:
1. Sends `mutableData: { fields: ['message.account'] }` to the wallet (lines 90-92), explicitly allowing the wallet to substitute `account` with a smart-wallet address.
2. Computes `permissionHash` from the wallet's response: `getHash({ permission: signResult.signedData.message, chainId })` (lines 125-128) — **correct**, uses post-substitution data.
3. Builds the returned object with `permission: typedData.message` (line 145) — **incorrect**, uses pre-substitution data.
When the wallet exercises the substitution (e.g., replaces the caller EOA `0xCaller` with a smart-wallet `0xSW`):
```
result.permissionHash → hash of { account: 0xSW, ... } // post-substitution
result.permission.account → 0xCaller // pre-substitution (WRONG)
```
The two fields now describe different permissions. Any downstream consumer that reads `result.permission.account` (e.g., to display to the user, to pass to `prepareSpendCallData`, or to call `fetchPermissions`) sees the wrong account address.
## Fix
Replace `typedData.message` with `signResult.signedData.message` in the object construction:
```diff
- permission: typedData.message,
+ permission: signResult.signedData.message,
```
This should be applied inside the `if (capabilities)` branch only (the `else` branch has no mutableData substitution and is correct as-is).
## Missing test coverage
The existing `wallet_sign` test (`'should use wallet_sign when capabilities are provided'`, line 277) mocks `signedData` as the same object as the original `typedData`, so no substitution actually occurs and the divergence is invisible. A test with `signedData.message.account` set to a **different** address than the request `account` would expose this.
## Non-security classification
This is a correctness/data-consistency bug, not a fund-drain vulnerability — the signature itself is produced by the wallet and remains valid. However, callers relying on `result.permission` for display, storage, or subsequent API calls will observe a wrong account field, which can cause silent failures downstream.
Contributor guide
Research direction
Start with packages/account-sdk/src/interface/public-utilities/spend-permission/methods/requestSpendPermission.ts, focusing on the capabilities branch and its existing wallet_sign test. Add regression coverage using a wallet-returned signedData.message.account that differs from the request account, then confirm the returned permission and permissionHash describe the same account.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- api, authorization
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 86/100