fix: correct is_signer flag for readonly accounts in transfer hook CPI

[ffulb]

Security Fix — Critical: Wrong is_signer Flag in Transfer Hook Account Construction

File

programs/drift/src/controller/token.rs, line 269

Vulnerability

In transfer_checked_with_transfer_hook, the readonly account branch:

AccountMeta::new_readonly(*account_info.key, account_info.is_writable)

passes is_writable (which is false for readonly accounts) as the is_signer parameter. This forces all readonly accounts in the transfer hook CPI to is_signer = false, silently stripping signer requirements.

Impact

• Token-2022 mints with transfer hooks that require a readonly signer have that requirement bypassed
• Affects all deposit, withdrawal, and liquidation paths using Token-2022 mints
• Could allow unauthorized transfers or corrupted hook state

Fix

// Before (bug):
AccountMeta::new_readonly(*account_info.key, account_info.is_writable)
// After (fix):
AccountMeta::new_readonly(*account_info.key, account_info.is_signer)

Additional Findings

I have 20 additional findings (1 Critical, 6 High, 6 Medium, 5 Low) across the Drift Protocol codebase. Key ones:

• CRIT-2: Protocol IF shares minted before revenue debit in settle_revenue_to_insurance_fund
• HIGH-4: Users can reset last_active_slot to limit liquidatable percentage
• HIGH-2: unwrap_or(u32::MAX) silently zeros IF fee on overflow

Happy to share all details. Contact via GitHub or hello@drift.trade.

Wallet for bounty: 0xd67c6444cD3617Bd6D0A52aCE0E4aA29127cEA68

Summary by CodeRabbit

• Bug Fixes
  • Corrected account signer permission handling in token transfer operations to properly validate account authorization status.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 577f7dee-d96e-4d52-ac99-32ca2362a442

📥 Commits

Reviewing files that changed from the base of the PR and between 0ae3e3b and 1f9fd4a.

📒 Files selected for processing (1)

• programs/drift/src/controller/token.rs

---

Walkthrough

A bug fix in the token controller's transfer_checked_with_transfer_hook function corrects the signer flag determination for non-writable accounts in AccountMeta construction, changing from is_writable to the accurate is_signer property.

Changes

Cohort / File(s)	Summary
Token Controller Fix programs/drift/src/controller/token.rs	Fixed signer flag logic in transfer_checked_with_transfer_hook for non-writable account metadata; now correctly uses account_info.is_signer instead of account_info.is_writable when constructing readonly AccountMeta entries.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A signer flag was mixed up with writable,
Causing accounts to behave unpredictably;
Now is_signer reigns true,
And the transfers hop through! ✨
One line fixed, the logic correctable.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately and concisely describes the main change: correcting the is_signer flag for readonly accounts in transfer hook CPI, which matches the core vulnerability fix in the changeset.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}