Skip to content

Commit 6d2a378

Browse files
committed
Prevent OOM when close account ix included for swap
1 parent 8be7a58 commit 6d2a378

3 files changed

Lines changed: 242 additions & 52 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1616
- program: prevent user from enabling HLM when they are failing maintenance margin check [#2116](https://github.com/drift-labs/protocol-v2/pull/2116)
1717
- program: fix bug where users are stuck in liquidation status after completed liquidation [#2122](https://github.com/drift-labs/protocol-v2/pull/2122)
1818
- program: skip isolated positions when checking for cross margin bankruptcy [#2123](https://github.com/drift-labs/protocol-v2/pull/2123)
19+
- program: prevent OOM when close account ix included for swap [#2148](https://github.com/drift-labs/protocol-v2/pull/2148)
1920

2021
### Breaking
2122

programs/drift/src/instructions/user.rs

Lines changed: 216 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,8 @@ use crate::{controller, math};
120120
use crate::{load_mut, ExchangeStatus};
121121
use anchor_lang::solana_program::sysvar::instructions;
122122
use borsh::{BorshDeserialize, BorshSerialize};
123+
use solana_program::pubkey::PUBKEY_BYTES;
124+
use solana_program::serialize_utils;
123125
use solana_program::sysvar::instructions::ID as IX_ID;
124126

125127
use super::optional_accounts::get_high_leverage_mode_config;
@@ -3754,10 +3756,144 @@ pub fn handle_enable_user_high_leverage_mode<'c: 'info, 'info>(
37543756
Ok(())
37553757
}
37563758

3759+
// We intentionally parse the instructions sysvar with zero-copy byte slices here
3760+
// to avoid heap growth/OOM risk from repeatedly deserializing full `Instruction`s,
3761+
// especially when post-swap CloseAccount instructions are included.
3762+
const INSTRUCTION_ACCOUNT_META_SIZE: usize = 1 + PUBKEY_BYTES;
3763+
const INSTRUCTION_ACCOUNT_META_IS_WRITABLE_BIT: u8 = 1 << 1;
3764+
3765+
struct InstructionSysvarView<'a> {
3766+
program_id: Pubkey,
3767+
account_meta_bytes: &'a [u8],
3768+
account_metas_len: usize,
3769+
data: &'a [u8],
3770+
}
3771+
3772+
impl<'a> InstructionSysvarView<'a> {
3773+
fn accounts_len(&self) -> usize {
3774+
self.account_metas_len
3775+
}
3776+
3777+
fn account_meta_bytes_at(&self, index: usize) -> std::result::Result<&[u8], ProgramError> {
3778+
if index >= self.account_metas_len {
3779+
return Err(ProgramError::InvalidInstructionData);
3780+
}
3781+
3782+
let start = index
3783+
.checked_mul(INSTRUCTION_ACCOUNT_META_SIZE)
3784+
.ok_or(ProgramError::InvalidInstructionData)?;
3785+
let end = start
3786+
.checked_add(INSTRUCTION_ACCOUNT_META_SIZE)
3787+
.ok_or(ProgramError::InvalidInstructionData)?;
3788+
3789+
self.account_meta_bytes
3790+
.get(start..end)
3791+
.ok_or(ProgramError::InvalidInstructionData)
3792+
}
3793+
3794+
fn account_pubkey_bytes_at(&self, index: usize) -> std::result::Result<&[u8], ProgramError> {
3795+
let account_meta = self.account_meta_bytes_at(index)?;
3796+
Ok(&account_meta[1..])
3797+
}
3798+
3799+
fn account_meta_bytes_iter(&self) -> impl Iterator<Item = &[u8]> {
3800+
self.account_meta_bytes
3801+
.chunks_exact(INSTRUCTION_ACCOUNT_META_SIZE)
3802+
}
3803+
3804+
fn account_pubkey_equals(
3805+
&self,
3806+
index: usize,
3807+
key: &Pubkey,
3808+
) -> std::result::Result<bool, ProgramError> {
3809+
Ok(self.account_pubkey_bytes_at(index)? == key.as_ref())
3810+
}
3811+
}
3812+
3813+
fn read_u16_le(
3814+
instruction_sysvar_data: &[u8],
3815+
offset: &mut usize,
3816+
) -> std::result::Result<u16, ProgramError> {
3817+
serialize_utils::read_u16(offset, instruction_sysvar_data)
3818+
.map_err(|_| ProgramError::InvalidInstructionData)
3819+
}
3820+
3821+
fn read_pubkey(
3822+
instruction_sysvar_data: &[u8],
3823+
offset: &mut usize,
3824+
) -> std::result::Result<Pubkey, ProgramError> {
3825+
serialize_utils::read_pubkey(offset, instruction_sysvar_data)
3826+
.map_err(|_| ProgramError::InvalidInstructionData)
3827+
}
3828+
3829+
fn read_slice<'a>(
3830+
instruction_sysvar_data: &'a [u8],
3831+
offset: &mut usize,
3832+
len: usize,
3833+
) -> std::result::Result<&'a [u8], ProgramError> {
3834+
let end = offset
3835+
.checked_add(len)
3836+
.ok_or(ProgramError::InvalidInstructionData)?;
3837+
let slice = instruction_sysvar_data
3838+
.get(*offset..end)
3839+
.ok_or(ProgramError::InvalidInstructionData)?;
3840+
*offset = end;
3841+
Ok(slice)
3842+
}
3843+
3844+
fn load_instruction_sysvar_view_at<'a>(
3845+
index: usize,
3846+
instruction_sysvar_data: &'a [u8],
3847+
) -> std::result::Result<InstructionSysvarView<'a>, ProgramError> {
3848+
let mut offset = 0;
3849+
let num_instructions = read_u16_le(instruction_sysvar_data, &mut offset)? as usize;
3850+
if index >= num_instructions {
3851+
return Err(ProgramError::InvalidArgument);
3852+
}
3853+
3854+
offset = 2usize
3855+
.checked_add(
3856+
index
3857+
.checked_mul(2)
3858+
.ok_or(ProgramError::InvalidInstructionData)?,
3859+
)
3860+
.ok_or(ProgramError::InvalidInstructionData)?;
3861+
3862+
let ix_offset = read_u16_le(instruction_sysvar_data, &mut offset)? as usize;
3863+
3864+
let mut ix_read_offset = ix_offset;
3865+
let account_metas_len = read_u16_le(instruction_sysvar_data, &mut ix_read_offset)? as usize;
3866+
3867+
let account_meta_bytes_len = account_metas_len
3868+
.checked_mul(INSTRUCTION_ACCOUNT_META_SIZE)
3869+
.ok_or(ProgramError::InvalidInstructionData)?;
3870+
let account_meta_bytes = read_slice(
3871+
instruction_sysvar_data,
3872+
&mut ix_read_offset,
3873+
account_meta_bytes_len,
3874+
)?;
3875+
3876+
let program_id = read_pubkey(instruction_sysvar_data, &mut ix_read_offset)?;
3877+
3878+
let instruction_data_len = read_u16_le(instruction_sysvar_data, &mut ix_read_offset)? as usize;
3879+
let data = read_slice(
3880+
instruction_sysvar_data,
3881+
&mut ix_read_offset,
3882+
instruction_data_len,
3883+
)?;
3884+
3885+
Ok(InstructionSysvarView {
3886+
program_id,
3887+
account_meta_bytes,
3888+
account_metas_len,
3889+
data,
3890+
})
3891+
}
3892+
37573893
/// Checks if an instruction is a SPL Token CloseAccount targeting
37583894
/// one of the swap's token accounts.
37593895
fn is_token_close_account_for_swap_ix(
3760-
ix: &solana_program::instruction::Instruction,
3896+
ix: &InstructionSysvarView,
37613897
in_token_account: &Pubkey,
37623898
out_token_account: &Pubkey,
37633899
) -> bool {
@@ -3774,12 +3910,22 @@ fn is_token_close_account_for_swap_ix(
37743910
}
37753911

37763912
// The first account in CloseAccount is the account being closed
3777-
if ix.accounts.is_empty() {
3913+
if ix.accounts_len() == 0 {
37783914
return false;
37793915
}
37803916

3781-
let account_to_close = &ix.accounts[0].pubkey;
3782-
account_to_close == in_token_account || account_to_close == out_token_account
3917+
let first_account_meta = match ix.account_meta_bytes_at(0) {
3918+
Ok(account_meta) => account_meta,
3919+
Err(_) => return false,
3920+
};
3921+
3922+
let account_to_close = &first_account_meta[1..];
3923+
let is_in_token_account = account_to_close == in_token_account.as_ref();
3924+
if is_in_token_account {
3925+
return true;
3926+
}
3927+
3928+
account_to_close == out_token_account.as_ref()
37833929
}
37843930

37853931
#[access_control(
@@ -3919,27 +4065,35 @@ pub fn handle_begin_swap<'c: 'info, 'info>(
39194065
)?;
39204066

39214067
let ixs = ctx.accounts.instructions.as_ref();
4068+
validate!(
4069+
instructions::check_id(ixs.key),
4070+
ErrorCode::InvalidSwap,
4071+
"invalid instructions sysvar account"
4072+
)?;
4073+
39224074
let current_index = instructions::load_current_index_checked(ixs)? as usize;
4075+
let instruction_sysvar_data = ixs.try_borrow_data()?;
39234076

3924-
let current_ix = instructions::load_instruction_at_checked(current_index, ixs)?;
4077+
let current_ix = load_instruction_sysvar_view_at(current_index, &instruction_sysvar_data)?;
39254078
validate!(
39264079
current_ix.program_id == *ctx.program_id,
39274080
ErrorCode::InvalidSwap,
39284081
"SwapBegin must be a top-level instruction (cant be cpi)"
39294082
)?;
4083+
let drift_program_id = crate::id();
39304084

39314085
// The only other drift program allowed is SwapEnd
39324086
let mut index = current_index + 1;
39334087
let mut found_end = false;
39344088
loop {
3935-
let ix = match instructions::load_instruction_at_checked(index, ixs) {
4089+
let ix = match load_instruction_sysvar_view_at(index, &instruction_sysvar_data) {
39364090
Ok(ix) => ix,
39374091
Err(ProgramError::InvalidArgument) => break,
39384092
Err(e) => return Err(e.into()),
39394093
};
39404094

39414095
// Check that the drift program key is not used
3942-
if ix.program_id == crate::id() {
4096+
if ix.program_id == drift_program_id {
39434097
// must be the last ix -- this could possibly be relaxed
39444098
validate!(
39454099
!found_end,
@@ -3951,85 +4105,102 @@ pub fn handle_begin_swap<'c: 'info, 'info>(
39514105
// must be the SwapEnd instruction
39524106
let discriminator = crate::instruction::EndSwap::discriminator();
39534107
validate!(
3954-
ix.data[0..8] == discriminator,
4108+
ix.data.len() >= 8 && ix.data[0..8] == discriminator,
39554109
ErrorCode::InvalidSwap,
39564110
"last drift ix must be end of swap"
39574111
)?;
39584112

39594113
validate!(
3960-
ctx.accounts.user.key() == ix.accounts[1].pubkey,
4114+
ix.accounts_len() >= 11,
4115+
ErrorCode::InvalidSwap,
4116+
"SwapEnd instruction has insufficient accounts"
4117+
)?;
4118+
4119+
validate!(
4120+
ix.account_pubkey_equals(1, &ctx.accounts.user.key())?,
39614121
ErrorCode::InvalidSwap,
39624122
"the user passed to SwapBegin and End must match"
39634123
)?;
39644124

39654125
validate!(
3966-
ctx.accounts.authority.key() == ix.accounts[3].pubkey,
4126+
ix.account_pubkey_equals(3, &ctx.accounts.authority.key())?,
39674127
ErrorCode::InvalidSwap,
39684128
"the authority passed to SwapBegin and End must match"
39694129
)?;
39704130

39714131
validate!(
3972-
ctx.accounts.out_spot_market_vault.key() == ix.accounts[4].pubkey,
4132+
ix.account_pubkey_equals(4, &ctx.accounts.out_spot_market_vault.key())?,
39734133
ErrorCode::InvalidSwap,
39744134
"the out_spot_market_vault passed to SwapBegin and End must match"
39754135
)?;
39764136

39774137
validate!(
3978-
ctx.accounts.in_spot_market_vault.key() == ix.accounts[5].pubkey,
4138+
ix.account_pubkey_equals(5, &ctx.accounts.in_spot_market_vault.key())?,
39794139
ErrorCode::InvalidSwap,
39804140
"the in_spot_market_vault passed to SwapBegin and End must match"
39814141
)?;
39824142

39834143
validate!(
3984-
ctx.accounts.out_token_account.key() == ix.accounts[6].pubkey,
4144+
ix.account_pubkey_equals(6, &ctx.accounts.out_token_account.key())?,
39854145
ErrorCode::InvalidSwap,
39864146
"the out_token_account passed to SwapBegin and End must match"
39874147
)?;
39884148

39894149
validate!(
3990-
ctx.accounts.in_token_account.key() == ix.accounts[7].pubkey,
4150+
ix.account_pubkey_equals(7, &ctx.accounts.in_token_account.key())?,
39914151
ErrorCode::InvalidSwap,
39924152
"the in_token_account passed to SwapBegin and End must match"
39934153
)?;
39944154

39954155
validate!(
3996-
ctx.remaining_accounts.len() == ix.accounts.len() - 11,
4156+
ctx.remaining_accounts.len() == ix.accounts_len() - 11,
39974157
ErrorCode::InvalidSwap,
39984158
"begin and end ix must have the same number of accounts"
39994159
)?;
40004160

4001-
for i in 11..ix.accounts.len() {
4002-
validate!(
4003-
*ctx.remaining_accounts[i - 11].key == ix.accounts[i].pubkey,
4004-
ErrorCode::InvalidSwap,
4005-
"begin and end ix must have the same accounts. {}th account mismatch. begin: {}, end: {}",
4006-
i,
4007-
ctx.remaining_accounts[i - 11].key,
4008-
ix.accounts[i].pubkey
4009-
)?;
4010-
}
4011-
} else {
4012-
if found_end {
4013-
if ix.program_id == lighthouse::ID {
4014-
continue;
4015-
}
4016-
4017-
// Allow closing the swap's token accounts after end_swap
4018-
if is_token_close_account_for_swap_ix(
4019-
&ix,
4020-
&ctx.accounts.in_token_account.key(),
4021-
&ctx.accounts.out_token_account.key(),
4022-
) {
4023-
continue;
4024-
}
4025-
4026-
for meta in ix.accounts.iter() {
4161+
let start_offset = 11 * INSTRUCTION_ACCOUNT_META_SIZE;
4162+
let end_remaining_accounts_meta_bytes = ix
4163+
.account_meta_bytes
4164+
.get(start_offset..)
4165+
.ok_or(ProgramError::InvalidInstructionData)?;
4166+
4167+
for (i, (account_meta_bytes, begin_remaining_account)) in
4168+
end_remaining_accounts_meta_bytes
4169+
.chunks_exact(INSTRUCTION_ACCOUNT_META_SIZE)
4170+
.zip(ctx.remaining_accounts.iter())
4171+
.enumerate()
4172+
{
4173+
if &account_meta_bytes[1..] != begin_remaining_account.key.as_ref() {
4174+
let mut end_account_bytes = [0_u8; 32];
4175+
end_account_bytes.copy_from_slice(&account_meta_bytes[1..]);
40274176
validate!(
4028-
meta.is_writable == false,
4177+
false,
40294178
ErrorCode::InvalidSwap,
4030-
"instructions after swap end must not have writable accounts"
4179+
"begin and end ix must have the same accounts. {}th account mismatch. begin: {}, end: {}",
4180+
i + 11,
4181+
begin_remaining_account.key,
4182+
Pubkey::new_from_array(end_account_bytes)
40314183
)?;
40324184
}
4185+
}
4186+
} else {
4187+
if found_end {
4188+
let is_allowed_post_end_ix = ix.program_id == lighthouse::ID
4189+
|| is_token_close_account_for_swap_ix(
4190+
&ix,
4191+
&ctx.accounts.in_token_account.key(),
4192+
&ctx.accounts.out_token_account.key(),
4193+
);
4194+
4195+
if !is_allowed_post_end_ix {
4196+
for account_meta_bytes in ix.account_meta_bytes_iter() {
4197+
validate!(
4198+
account_meta_bytes[0] & INSTRUCTION_ACCOUNT_META_IS_WRITABLE_BIT == 0,
4199+
ErrorCode::InvalidSwap,
4200+
"instructions after swap end must not have writable accounts"
4201+
)?;
4202+
}
4203+
}
40334204
} else {
40344205
let mut whitelisted_programs = WHITELISTED_SWAP_PROGRAMS.to_vec();
40354206
if !delegate_is_signer {
@@ -4044,9 +4215,9 @@ pub fn handle_begin_swap<'c: 'info, 'info>(
40444215
"only allowed to pass in ixs to ATA, openbook, Jupiter v3/v4/v6, dflow, or titan programs"
40454216
)?;
40464217

4047-
for meta in ix.accounts.iter() {
4218+
for account_meta_bytes in ix.account_meta_bytes_iter() {
40484219
validate!(
4049-
meta.pubkey != crate::id(),
4220+
&account_meta_bytes[1..] != drift_program_id.as_ref(),
40504221
ErrorCode::InvalidSwap,
40514222
"instructions between begin and end must not be drift instructions"
40524223
)?;

0 commit comments

Comments
 (0)