On a $20M Bug in Jet Protocol
Recently, Charlie You disclosed a vulnerability in the Jet Protocol (see tweet). The vulnerability would have caused a $20m loss of Jet users’ funds if exploited. Fortunately, Jet patched it before any user was affected.
The sec3 team identified something tricky in Jet-v1’s code and had a discussion with Charlie shortly after the disclosure. It turns out that the vulnerability has a different cause (unexpected by Charlie). Here is a summary:
-
The cause is actually an unvalidated input
account, and it would have been detected by sec3 X-RayAuto Auditor easily. -
The vulnerability was introduced in commit 6133de on Dec 15, 2021, but not exploitable.
-
The vulnerability became exploitable because of a follow-up commit 1f7da4 on Dec 21, 2021.
-
The exploitable window spanned over five weeks from Dec 21 to Jan 27.
-
Anchor works in the correct way as we expect.
Details
1. The real vulnerability
In commit 6133de, Jet added a file withdraw_tokens.rs, which defines a function handler :
88 + /// Withdraw tokens from a reserve
89 + pub fn handler(ctx: Context<WithdrawTokens>, amount: Amount) -> ProgramResult {
90 + let market = ctx.accounts.market.load()?;
91 + let mut reserve = ctx.accounts.reserve.load_mut()?;The function does two things:
- Transfer tokens from the reserve to the user’s
withdraw_account
103 + // Transfer the tokens from the reserve, and burn
104 + token::transfer(
105 + ctx.accounts
106 + .transfer_context()
107 + .with_signer(&[&market.authority_seeds()])
108 + token_amount,- Burn notes from the user’s provided
deposit_note_account
111 + token::burn(
112 + ctx.accounts
113 + .note_burn_context()
114 + .with_signer(&[&market.authority_seeds()])
115 + note_amount,The function note_burn_context creates a Burn instruction on the deposit_note_account (line 80) with market_authority (line 82):
76 + fn note_burn_context(&self) -> CpiContext<'_, '_, '_, 'info, Burn<'info>> {
77 + CpiContext::new(
78 + self.token_program.clone(),
79 + Burn {
80 + to: self.deposit_note_account.to_account_info(),
81 + mint: self.deposit_note_mint.to_account_info(),
82 + authority: self.market_authority.clone(),The real vulnerability lies in that deposit_note_account is an unvalidated input account (line 54):
48 + /// The user/authority that owns the deposit
49 + #[account(signer)]
50 + pub depositor: AccountInfo<'info>,
51 +
52 + /// The account that stores the deposit notes
53 + #[account(mut)]
54 + pub deposit_note_account: AccountInfo<'info>,The market_authority is the authority of all users’ deposit_account (code):
/// The account that will store the deposit notes
#[account(init,
seeds = [
b"deposits".as_ref(),
reserve.key().as_ref(),
depositor.key.as_ref()
],
bump = bump,
token::mint = deposit_note_mint,
token::authority = market_authority,
payer = depositor)]
pub deposit_account: AccountInfo<'info>,Therefore, an attack may supply any user’s deposit_note_account and burn notes successfully.
2. The vulnerability was not (yet) exploitable
However, the vulnerability is not exploitable because the withdraw_tokens::handler function in withdraw_tokens.rs is not an attack surface: it cannot be directly called by users.
It can only be called from the handler function of withdraw.rs (line 74):
74 + super::withdraw_tokens::handler(
75 + Context::new(
76 + ctx.program_id,
77 + &mut super::withdraw_tokens::WithdrawTokens::try_accounts(
78 + ctx.program_id,
79 + &mut &&*ctx.accounts.to_account_infos(),
80 + &[],
81 + )?,
82 + &[],The withdraw::handler function is an attack surface (via the Withdraw instruction):
72 /// Withdraw tokens from a reserve
73 pub fn handler(ctx: Context<Withdraw>, _bump: u8, amount: Amount) -> ProgramResult {However, the deposit_account in the Withdraw instruction is properly validated:
50 /// The user/authority that owns the deposit
51 #[account(signer)]
52 pub depositor: AccountInfo<'info>,
53
54 /// The account that stores the deposit notes
55 #[account(mut,
56 seeds = [
57 b"deposits".as_ref(),
58 reserve.key().as_ref(),
59 depositor.key.as_ref()
60 ],
61 bump = bump)]
62 pub deposit_account: AccountInfo<'info>,In the above, depositor is signer, and deposit_account is unique to the depositor (depositor.key is part of the PDA seeds).
deposit_account is passed as deposit_note_account to withdraw_tokens::handler.
Therefore, attackers cannot exploit the vulnerability because deposit_account is validated.
3. When the vulnerability became exploitable
In commit 1f7da4, Jet introduced the WithdrawTokens instruction, which sets the withdraw_tokens::handler function as an attack surface:
140 + /// Withdraw tokens from a reserve (unmanaged)
141 + pub fn withdraw_tokens(ctx: Context<WithdrawTokens>, amount: Amount) -> ProgramResult {
142 + instructions::withdraw_tokens::handler(ctx, amount)Thereafter, the vulnerability became directly exploitable.
4. When and how the vulnerability was fixed
On Jan 27, the vulnerability was fixed in commit-f4bb3b by changing the authority to depositor (line 82):
76 fn note_burn_context(&self) -> CpiContext<'_, '_, '_, 'info, Burn<'info>> {
77 CpiContext::new(
78 self.token_program.clone(),
79 accounts: Burn {
80 to: self.deposit_note_account.to_account_info(),
81 mint: self.deposit_note_mint.to_account_info(),
82 authority: self.depositor.clone(),The depositor is signer:
48 /// The user/authority that owns the deposit
49 #[account(signer)]
50 pub depositor: AccountInfo<'info>,This ensures that an attacker cannot create a fake depositor to invoke WithdrawTokens successfully.
In fact, the only way to call withdraw_tokens::handler successfully is through the Withdraw instruction. The withdraw::handler function creates a CPI to call the WithdrawTokens instruction (line 108):
98 /// Withdraw tokens from a reserve
99 pub fn handler(ctx: Context<Withdraw>, _bump: u8, amount: Amount) -> ProgramResult {
100 let market: Ref<Market> = ctx.accounts.market.load()?;
101 let wt_ctx: CpiContext<WithdrawTokens> = ctx.accounts.withdraw_tokens_context();
102 let ix: Instruction = Instruction {
103 program_id: crate::ID,
104 accounts: wt_ctx.to_account_metas( is_signer: Some(true)),
105 data: crate::instruction::WithdrawTokens { amount }.data(),
106 };
107
108 invoke_signed(
109 instruction: &ix,
110 &wt_ctx.to_account_infos(),
111 signers_seeds: &[&market.authority_seeds()],The CPI passes market_authority as the depositor (line 89), which will be used as the authority for the Burn instruction:
80 fn withdraw_tokens_context(&self) -> CpiContext<'_, '_, '_, 'info, WithdrawTokens<'info>> {
81 CpiContext::new(
82 program: self.jet_program.to_account_info(),
83 accounts: WithdrawTokens {
84 market: self.market.to_account_info(),
85 market_authority: self.market_authority.to_account_info(),
86 reserve: self.reserve.to_account_info(),
87 vault: self.vault.to_account_info(),
88 deposit_note_mint: self.deposit_note_mint.to_account_info(),
89 depositor: self.market_authority.to_account_info(),
90 deposit_note_account: self.deposit_account.to_account_info(),
91 withdraw_account: self.withdraw_account.to_account_info(),
92 token_program: self.token_program.clone(),5. Anchor works in the correct way as we expect
Initially, we suspected that the vulnerability might be due to additional attack surfaces generated by Anchor accidentally. For instance, whether Anchor-generated attack surfaces not defined in the #program macro, but imported by instructions::\* or use super::\* :
31 use instructions::*;
32 use state::*;
33
34 declare_id!("JPv1rCqrhagNNmJVM5J1he7msQ5ybtvE1nNuHpDHMNU");
35
36 pub const LIQUIDATE_DEX_INSTR_ID: [u8; 8] = [28, 129, 253, 125, 243, 52, 11, 162];
37
38 #[program]
39 mod jet {
40 use super::*;However, after close inspection, we confirmed that Anchor works as what we expected: its generated code only contains surfaces defined in the #program macro.
How X-Ray detects the vulnerability
X-Ray Auto Auditor can detect this vulnerability automatically (i.e., deposit_note_account is an unvalidated input account). The following shows a screenshot of the report (line 54):
****************** attack surface #17: sol.withdraw_tokens ******************
isOwnerOnly: 0
account: deposit_note_account
==============VULNERABLE: UnvalidatedAccount!==============
Found a potential vulnerability at line 54, column 8 in src/instructions/withdraw_tokens.rs
The account may not be properly validated and may be untrustful:
48| /// The user/authority that owns the deposit
49| #[account(signer)]
50| pub depositor: AccountInfo<'info>,
51|
52| /// The account that stores the deposit notes
53| #[account(mut)]
>54| pub deposit_note_account: AccountInfo<'info>,
55|
56| /// The token account where to transfer withdrawn tokens to
57| #[account(mut)]
58| pub withdraw_account: AccountInfo<'info>,
59|
60| #[account(address = token::ID)]
>>>Stack Trace:
For more info, see https://medium.com/coinmonks/the-wormhole-hack-how-soteria-detects-the-v...
The unvalidated input account vulnerability introduced in commit 3178f93.
The unvalidated input account vulnerability introduced in commit 3178f93.
Inspired by Charlie’s original tweet, sec3 team also added a new vulnerability signature to the SVE database:
- SVE1032: IncorrectAuthorityAccount — The PDA operation may be incorrectly used as shared authority and may allow any account to transfer or burn tokens.
"1032": {
"name": "IncorrectAuthorityAccount",
"description": "The PDA account may be incorrectly used as a shared authority and may allow any account to transfer or burn tokens.",
"url": "https://twitter.com/CharlieYouAI/status/1508842093514567687"
},The algorithm is actually simple: (1) if any PDA account (e.g., market_authority) is used as authority to transfer/mint/burn tokens, and (2) if the from/to account in the transfer/mint/burn instruction is not connected with a signer account (e.g., PDA seeds).
If both conditions are true, a potential vulnerability will be flagged.
The following shows a screenshot of the IncorrectAuthorityAccount report:
==============VULNERABLE: IncorrectAuthorityAccount!==============
Found a potential vulnerability at line 33, column 8 in src/instructions/withdraw_tokens.rs
The PDA account may be incorrectly used as a shared authority and may allow any account to transfer or burn tokens:
27|pub struct Withdraw<'info> {
28| /// The relevant market this withdraw is for
29| #[account(has_one = market_authority)]
30| pub market: Loader<'info, Market>,
31|
32| /// The market's authority account
>33| pub market_authority: AccountInfo<'info>,
34|
35| /// The reserve being withdrawn from
36| #[account(mut,
37| has_one = market,
38| has_one = vault,
39| has_one = deposit_note_mint)]
>>>Stack Trace:
For more info, see https://twitter.com/CharlieYouAI/status/1508842093514567687
In conclusion, (1) sec3's tool would’ve detected this vulnerability, (2) sec3 learned a new vulnerability signature from this incident, and added to the tool.h
sec3 Audit
sec3 (formerly Soteria) is founded by leading minds in the fields of blockchain security and software verification.
We are pleased to provide full audit services to high-impact Dapps on Solana. Please visit sec3.dev or email contact@sec3.dev