Skip to content

Commit a025498

Browse files
vaporifvaporif
authored
fix(solana): owner/checked validations (#819)
1 parent 28ca34b commit a025498

File tree

16 files changed

+213
-122
lines changed

16 files changed

+213
-122
lines changed

‎programs/solana/programs/ics07-tendermint/src/error.rs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,4 +71,8 @@ pub enum ErrorCode {
7171
SerializationError,
7272
#[msg("Account validation failed: invalid account or PDA")]
7373
AccountValidationFailed,
74+
#[msg("Invalid account owner: account is not owned by the expected program")]
75+
InvalidAccountOwner,
76+
#[msg("Arithmetic overflow detected")]
77+
ArithmeticOverflow,
7478
}

‎programs/solana/programs/ics07-tendermint/src/instructions/assemble_and_submit_misbehaviour.rs‎

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -60,13 +60,20 @@ fn validate_and_load_chunk(
6060
&[index],
6161
];
6262
let (expected_pda, _) = Pubkey::find_program_address(expected_seeds, &crate::ID);
63-
require_eq!(
63+
require_keys_eq!(
6464
chunk_account.key(),
6565
expected_pda,
6666
ErrorCode::InvalidChunkAccount
6767
);
6868

69+
require_keys_eq!(
70+
*chunk_account.owner,
71+
crate::ID,
72+
ErrorCode::InvalidAccountOwner
73+
);
74+
6975
let chunk_data = chunk_account.try_borrow_data()?;
76+
7077
let chunk: MisbehaviourChunk = MisbehaviourChunk::try_deserialize(&mut &chunk_data[..])?;
7178

7279
misbehaviour_bytes.extend_from_slice(&chunk.chunk_data);
@@ -106,12 +113,15 @@ fn process_misbehaviour(
106113
)
107114
.map_err(|_| error!(ErrorCode::MisbehaviourCheckFailed))?;
108115

109-
require!(
110-
ctx.accounts.trusted_consensus_state_1.height == output.trusted_height_1.revision_height(),
116+
require_eq!(
117+
ctx.accounts.trusted_consensus_state_1.height,
118+
output.trusted_height_1.revision_height(),
111119
ErrorCode::HeightMismatch
112120
);
113-
require!(
114-
ctx.accounts.trusted_consensus_state_2.height == output.trusted_height_2.revision_height(),
121+
122+
require_eq!(
123+
ctx.accounts.trusted_consensus_state_2.height,
124+
output.trusted_height_2.revision_height(),
115125
ErrorCode::HeightMismatch
116126
);
117127

@@ -140,19 +150,27 @@ fn cleanup_chunks(
140150
&[index as u8],
141151
];
142152
let (expected_pda, _) = Pubkey::find_program_address(expected_seeds, &crate::ID);
143-
require_eq!(
153+
require_keys_eq!(
144154
chunk_account.key(),
145155
expected_pda,
146156
ErrorCode::InvalidChunkAccount
147157
);
148158

159+
require_keys_eq!(
160+
*chunk_account.owner,
161+
crate::ID,
162+
ErrorCode::InvalidAccountOwner
163+
);
164+
149165
let mut data = chunk_account.try_borrow_mut_data()?;
150166
data.fill(0);
151167

152168
let mut lamports = chunk_account.try_borrow_mut_lamports()?;
153169
let mut submitter_lamports = ctx.accounts.submitter.try_borrow_mut_lamports()?;
154170

155-
**submitter_lamports += **lamports;
171+
**submitter_lamports = submitter_lamports
172+
.checked_add(**lamports)
173+
.ok_or(ErrorCode::ArithmeticOverflow)?;
156174
**lamports = 0;
157175
}
158176
Ok(())

‎programs/solana/programs/ics07-tendermint/src/instructions/assemble_and_update_client.rs‎

Lines changed: 40 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -42,47 +42,34 @@ fn assemble_chunks(
4242
let mut header_bytes = Vec::new();
4343

4444
for (index, chunk_account) in ctx.remaining_accounts.iter().enumerate() {
45-
validate_and_load_chunk(
46-
chunk_account,
47-
chain_id,
48-
target_height,
49-
submitter,
50-
index as u8,
51-
&mut header_bytes,
52-
)?;
53-
}
45+
let expected_seeds = &[
46+
crate::state::HeaderChunk::SEED,
47+
submitter.as_ref(),
48+
chain_id.as_bytes(),
49+
&target_height.to_le_bytes(),
50+
&[index as u8],
51+
];
52+
let (expected_pda, _) = Pubkey::find_program_address(expected_seeds, &crate::ID);
53+
require_keys_eq!(
54+
chunk_account.key(),
55+
expected_pda,
56+
ErrorCode::InvalidChunkAccount
57+
);
5458

55-
Ok(header_bytes)
56-
}
59+
require_keys_eq!(
60+
*chunk_account.owner,
61+
crate::ID,
62+
ErrorCode::InvalidAccountOwner
63+
);
5764

58-
fn validate_and_load_chunk(
59-
chunk_account: &AccountInfo,
60-
chain_id: &str,
61-
target_height: u64,
62-
submitter: Pubkey,
63-
index: u8,
64-
header_bytes: &mut Vec<u8>,
65-
) -> Result<()> {
66-
// Validate chunk PDA
67-
let expected_seeds = &[
68-
crate::state::HeaderChunk::SEED,
69-
submitter.as_ref(),
70-
chain_id.as_bytes(),
71-
&target_height.to_le_bytes(),
72-
&[index],
73-
];
74-
let (expected_pda, _) = Pubkey::find_program_address(expected_seeds, &crate::ID);
75-
require_eq!(
76-
chunk_account.key(),
77-
expected_pda,
78-
ErrorCode::InvalidChunkAccount
79-
);
65+
let chunk_data = chunk_account.try_borrow_data()?;
8066

81-
let chunk_data = chunk_account.try_borrow_data()?;
82-
let chunk: HeaderChunk = HeaderChunk::try_deserialize(&mut &chunk_data[..])?;
67+
let chunk: HeaderChunk = HeaderChunk::try_deserialize(&mut &chunk_data[..])?;
8368

84-
header_bytes.extend_from_slice(&chunk.chunk_data);
85-
Ok(())
69+
header_bytes.extend_from_slice(&chunk.chunk_data);
70+
}
71+
72+
Ok(header_bytes)
8673
}
8774

8875
fn process_header_update(
@@ -176,19 +163,27 @@ fn cleanup_chunks(
176163
&[index as u8],
177164
];
178165
let (expected_pda, _) = Pubkey::find_program_address(expected_seeds, &crate::ID);
179-
require_eq!(
166+
require_keys_eq!(
180167
chunk_account.key(),
181168
expected_pda,
182169
ErrorCode::InvalidChunkAccount
183170
);
184171

172+
require_keys_eq!(
173+
*chunk_account.owner,
174+
crate::ID,
175+
ErrorCode::InvalidAccountOwner
176+
);
177+
185178
let mut data = chunk_account.try_borrow_mut_data()?;
186179
data.fill(0);
187180

188181
let mut lamports = chunk_account.try_borrow_mut_lamports()?;
189182
let mut submitter_lamports = ctx.accounts.submitter.try_borrow_mut_lamports()?;
190183

191-
**submitter_lamports += **lamports;
184+
**submitter_lamports = submitter_lamports
185+
.checked_add(**lamports)
186+
.ok_or(ErrorCode::ArithmeticOverflow)?;
192187
**lamports = 0;
193188
}
194189
Ok(())
@@ -210,8 +205,9 @@ fn load_consensus_state(
210205
&crate::ID,
211206
);
212207

213-
require!(
214-
expected_pda == account.key(),
208+
require_keys_eq!(
209+
account.key(),
210+
expected_pda,
215211
ErrorCode::AccountValidationFailed
216212
);
217213

@@ -244,8 +240,9 @@ fn store_consensus_state(params: StoreConsensusStateParams) -> Result<UpdateResu
244240
&crate::ID,
245241
);
246242

247-
require!(
248-
expected_pda == params.account.key(),
243+
require_keys_eq!(
244+
expected_pda,
245+
params.account.key(),
249246
ErrorCode::AccountValidationFailed
250247
);
251248

‎programs/solana/programs/ics07-tendermint/src/instructions/cleanup_incomplete_misbehaviour.rs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,9 @@ pub fn cleanup_incomplete_misbehaviour(
2828
let mut lamports = chunk_account.try_borrow_mut_lamports()?;
2929
let mut submitter_lamports =
3030
ctx.accounts.submitter_account.try_borrow_mut_lamports()?;
31-
**submitter_lamports += **lamports;
31+
**submitter_lamports = submitter_lamports
32+
.checked_add(**lamports)
33+
.ok_or(crate::error::ErrorCode::ArithmeticOverflow)?;
3234
**lamports = 0;
3335
}
3436
}

‎programs/solana/programs/ics07-tendermint/src/instructions/cleanup_incomplete_upload.rs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,9 @@ pub fn cleanup_incomplete_upload(
3939
let mut lamports = chunk_account.try_borrow_mut_lamports()?;
4040
let mut submitter_lamports =
4141
ctx.accounts.submitter_account.try_borrow_mut_lamports()?;
42-
**submitter_lamports += **lamports;
42+
**submitter_lamports = submitter_lamports
43+
.checked_add(**lamports)
44+
.ok_or(crate::error::ErrorCode::ArithmeticOverflow)?;
4345
**lamports = 0;
4446
}
4547
// If account doesn't exist or isn't owned by us, skip it

‎programs/solana/programs/ics26-router/src/errors.rs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,9 @@ pub enum RouterError {
7878

7979
#[msg("Direct calls not allowed, must be called via CPI")]
8080
DirectCallNotAllowed,
81+
82+
#[msg("Invalid account owner: account is not owned by the expected program")]
83+
InvalidAccountOwner,
8184
}
8285

8386
/// Convert CPI validation errors to Router errors

‎programs/solana/programs/ics26-router/src/instructions/ack_packet.rs‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -87,18 +87,21 @@ pub fn ack_packet<'info>(
8787
let packet_commitment_account = &ctx.accounts.packet_commitment;
8888
let client = &ctx.accounts.client;
8989

90-
require!(
91-
ctx.accounts.relayer.key() == router_state.authority,
90+
require_keys_eq!(
91+
ctx.accounts.relayer.key(),
92+
router_state.authority,
9293
RouterError::UnauthorizedSender
9394
);
9495

95-
require!(
96-
msg.packet.source_client == client.client_id,
96+
require_eq!(
97+
&msg.packet.source_client,
98+
&client.client_id,
9799
RouterError::ClientMismatch
98100
);
99101

100-
require!(
101-
msg.packet.dest_client == client.counterparty_info.client_id,
102+
require_eq!(
103+
&msg.packet.dest_client,
104+
&client.counterparty_info.client_id,
102105
RouterError::InvalidCounterpartyClient
103106
);
104107

@@ -116,8 +119,9 @@ pub fn ack_packet<'info>(
116119
let (expected_ibc_app, _) =
117120
Pubkey::find_program_address(&[IBCApp::SEED, payload.source_port.as_bytes()], &crate::ID);
118121

119-
require!(
120-
ctx.accounts.ibc_app.key() == expected_ibc_app,
122+
require_keys_eq!(
123+
ctx.accounts.ibc_app.key(),
124+
expected_ibc_app,
121125
RouterError::IbcAppNotFound
122126
);
123127

‎programs/solana/programs/ics26-router/src/instructions/add_ibc_app.rs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,9 @@ pub fn add_ibc_app(ctx: Context<AddIbcApp>, port_id: String) -> Result<()> {
3737
let router_state = &ctx.accounts.router_state;
3838
let ibc_app = &mut ctx.accounts.ibc_app;
3939

40-
require!(
41-
ctx.accounts.authority.key() == router_state.authority,
40+
require_keys_eq!(
41+
ctx.accounts.authority.key(),
42+
router_state.authority,
4243
RouterError::UnauthorizedSender
4344
);
4445

‎programs/solana/programs/ics26-router/src/instructions/cleanup_chunks.rs‎

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ pub fn cleanup_chunks<'info>(
2727

2828
let chunk_account = &ctx.remaining_accounts[chunk_index];
2929

30-
// Verify the PDA is correct
3130
let expected_seeds = &[
3231
PayloadChunk::SEED,
3332
relayer_key.as_ref(),
@@ -38,11 +37,18 @@ pub fn cleanup_chunks<'info>(
3837
];
3938
let (expected_pda, _) = Pubkey::find_program_address(expected_seeds, &crate::ID);
4039

41-
require!(
42-
chunk_account.key() == expected_pda,
40+
require_keys_eq!(
41+
chunk_account.key(),
42+
expected_pda,
4343
RouterError::InvalidChunkAccount
4444
);
4545

46+
require_keys_eq!(
47+
*chunk_account.owner,
48+
crate::ID,
49+
RouterError::InvalidAccountOwner
50+
);
51+
4652
// Return rent to relayer
4753
cleanup_single_chunk(chunk_account, &ctx.accounts.relayer)?;
4854
chunk_index += 1;
@@ -58,7 +64,6 @@ pub fn cleanup_chunks<'info>(
5864

5965
let chunk_account = &ctx.remaining_accounts[chunk_index];
6066

61-
// Verify the PDA is correct
6267
let expected_seeds = &[
6368
ProofChunk::SEED,
6469
relayer_key.as_ref(),
@@ -68,11 +73,18 @@ pub fn cleanup_chunks<'info>(
6873
];
6974
let (expected_pda, _) = Pubkey::find_program_address(expected_seeds, &crate::ID);
7075

71-
require!(
72-
chunk_account.key() == expected_pda,
76+
require_keys_eq!(
77+
chunk_account.key(),
78+
expected_pda,
7379
RouterError::InvalidChunkAccount
7480
);
7581

82+
require_keys_eq!(
83+
*chunk_account.owner,
84+
crate::ID,
85+
RouterError::InvalidAccountOwner
86+
);
87+
7688
// Return rent to relayer
7789
cleanup_single_chunk(chunk_account, &ctx.accounts.relayer)?;
7890
chunk_index += 1;

‎programs/solana/programs/ics26-router/src/instructions/recv_packet.rs‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -108,18 +108,21 @@ pub fn recv_packet<'info>(
108108
// Get clock directly via syscall
109109
let clock = Clock::get()?;
110110

111-
require!(
112-
ctx.accounts.relayer.key() == router_state.authority,
111+
require_keys_eq!(
112+
ctx.accounts.relayer.key(),
113+
router_state.authority,
113114
RouterError::UnauthorizedSender
114115
);
115116

116-
require!(
117-
msg.packet.source_client == client.counterparty_info.client_id,
117+
require_eq!(
118+
&msg.packet.source_client,
119+
&client.counterparty_info.client_id,
118120
RouterError::InvalidCounterpartyClient
119121
);
120122

121-
require!(
122-
msg.packet.dest_client == client.client_id,
123+
require_eq!(
124+
&msg.packet.dest_client,
125+
&client.client_id,
123126
RouterError::ClientMismatch
124127
);
125128

@@ -143,8 +146,9 @@ pub fn recv_packet<'info>(
143146
let (expected_ibc_app, _) =
144147
Pubkey::find_program_address(&[IBCApp::SEED, payload.dest_port.as_bytes()], &crate::ID);
145148

146-
require!(
147-
ctx.accounts.ibc_app.key() == expected_ibc_app,
149+
require_keys_eq!(
150+
ctx.accounts.ibc_app.key(),
151+
expected_ibc_app,
148152
RouterError::IbcAppNotFound
149153
);
150154

0 commit comments

Comments
 (0)