Skip to content

fix: pin spend permission spender when routing through global account#350

Merged
fan-zhang-sv merged 1 commit into
masterfrom
fix/spend-permission-spender-pin
Jun 26, 2026
Merged

fix: pin spend permission spender when routing through global account#350
fan-zhang-sv merged 1 commit into
masterfrom
fix/spend-permission-spender-pin

Conversation

@fan-zhang-sv

Copy link
Copy Markdown
Collaborator

Summary

  • Strip dApp-supplied spendPermissions before routing sendCalls through the global account.
  • Preserve unrelated request capabilities and inject the SDK-pinned sub-account spender.
  • Add a regression test for attacker-supplied spend permission spender override.

Test plan

  • yarn workspace @base-org/account test --run src/sign/base-account/utils/routeThroughGlobalAccount.test.ts
  • Cursor lints on touched files

Made with Cursor

Co-authored-by: Cursor <cursoragent@cursor.com>
@cb-heimdall

cb-heimdall commented Jun 26, 2026

Copy link
Copy Markdown
Collaborator

✅ Heimdall Review Status

Requirement Status More Info
Reviews 2/1
Denominator calculation
Show calculation
1 if user is bot 0
1 if user is external 0
2 if repo is sensitive 0
From .codeflow.yml 1
Additional review requirements
Show calculation
Max 0
0
From CODEOWNERS 0
Global minimum 0
Max 1
1
1 if commit is unverified 0
Sum 1

@github-actions

Copy link
Copy Markdown
Contributor

Review Summary

PR Title: Conforms to Conventional Commits ✓

Security fix assessment: This is a well-targeted security patch. Without the fix, a malicious dApp could inject spendPermissions: { request: { spender: attackerAddress } } into wallet_sendCalls capabilities. Because injectRequestCapabilities gives precedence to request-provided capabilities (request spreads second in the shallow merge), the attacker-supplied spender would override the SDK-pinned subAccountAddress, granting the spend permission to an arbitrary address.

Implementation: Correct. The fix:

  1. Destructures capabilities out of the dApp-controlled params
  2. Shallow-copies and deletes spendPermissions from the copy
  3. Only passes remaining safe capabilities through to the reconstructed request
  4. Lets injectRequestCapabilities inject the SDK-controlled spender as intended

No issues found. The fix is minimal, correctly scoped, and includes a regression test that verifies the attack vector is closed.

@fan-zhang-sv
fan-zhang-sv merged commit 1009130 into master Jun 26, 2026
12 checks passed
@fan-zhang-sv
fan-zhang-sv deleted the fix/spend-permission-spender-pin branch June 26, 2026 18:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants