base / base/account-sdk

spend-permission: requestSpendPermission returns inconsistent SpendPermission when wallet substitutes account via mutableData

Open Beginner friendly
#324 0 comments 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.