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 function does two things:
1. Transfer tokens from the reserve to the user’s withdraw_account
2. Burn notes from the user’s provided deposit_note_account
The function note_burn_context creates a Burn instruction on the deposit_note_account (line 80) with market_authority (line 82):
The real vulnerability lies in that deposit_note_account is an unvalidated input account (line 54):
The market_authority is the authority of all users’ deposit_account (code):
Therefore, an attack may supply any user’s deposit_note_account and burn notes successfully.
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):
The withdraw::handler function is an attack surface (via the Withdraw instruction):
However, the deposit_account in the Withdraw instruction is properly validated:
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.
In commit 1f7da4, Jet introduced the WithdrawTokens instruction, which sets the withdraw_tokens::handler function as an attack surface:
Thereafter, the vulnerability became directly exploitable.
On Jan 27, the vulnerability was fixed in commit-f4bb3b by changing the authority to depositor (line 82):
The depositor is signer:
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):
The CPI passes market_authority as the depositor (line 89), which will be used as the authority for the Burn instruction:
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::* :
However, after close inspection, we confirmed that Anchor works as what we expected: its generated code only contains surfaces defined in the #program macro.
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):
Inspired by Charlie’s original tweet, sec3 team also added a new vulnerability signature to the SVE database:
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:
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 (formerly Soteria) is founded by leading minds in the fields of blockchain security and software verification.