Skip to content

Commit 97d5a33

Browse files
onspeedhponspeedhp
committed
fix(audit-5): use Rent sysvar instead of hardcoded rent calculations
- Replace hardcoded rent formula (897840 + space * 6960) with Rent::from_account_info() and rent.minimum_balance() in create_wallet.rs and create_session.rs - Add Rent sysvar account to instruction accounts in both processors - Update SDK instructions.ts to include Rent sysvar account - Update all E2E tests to pass Rent sysvar account Fixes #5 - Hardcoded rent calculations might go out of sync after chain update
1 parent 9c6c265 commit 97d5a33

10 files changed

Lines changed: 39 additions & 24 deletions

File tree

‎Cargo.lock‎

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎program/src/processor/create_session.rs‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ use pinocchio::{
66
program::invoke_signed,
77
program_error::ProgramError,
88
pubkey::{find_program_address, Pubkey},
9+
sysvars::rent::Rent,
910
ProgramResult,
1011
};
1112

@@ -65,6 +66,7 @@ impl CreateSessionArgs {
6566
/// 3. `[signer, writable]` Authorizer: Authority approving this session creation.
6667
/// 4. `[writable]` Session PDA: The new session account.
6768
/// 5. `[]` System Program.
69+
/// 6. `[]` Rent Sysvar.
6870
pub fn process(
6971
program_id: &Pubkey,
7072
accounts: &[AccountInfo],
@@ -88,6 +90,12 @@ pub fn process(
8890
let system_program = account_info_iter
8991
.next()
9092
.ok_or(ProgramError::NotEnoughAccountKeys)?;
93+
let rent_sysvar = account_info_iter
94+
.next()
95+
.ok_or(ProgramError::NotEnoughAccountKeys)?;
96+
97+
// Get rent from sysvar (fixes audit issue #5 - hardcoded rent calculations)
98+
let rent = Rent::from_account_info(rent_sysvar)?;
9199

92100
if wallet_pda.owner() != program_id || authorizer_pda.owner() != program_id {
93101
return Err(ProgramError::IllegalOwner);
@@ -165,15 +173,11 @@ pub fn process(
165173

166174
// Create Session Account
167175
let space = std::mem::size_of::<SessionAccount>();
168-
// Rent: 897840 + (space * 6960)
169-
let rent = (space as u64)
170-
.checked_mul(6960)
171-
.and_then(|val| val.checked_add(897840))
172-
.ok_or(ProgramError::ArithmeticOverflow)?;
176+
let session_rent = rent.minimum_balance(space);
173177

174178
let mut create_ix_data = Vec::with_capacity(52);
175179
create_ix_data.extend_from_slice(&0u32.to_le_bytes());
176-
create_ix_data.extend_from_slice(&rent.to_le_bytes());
180+
create_ix_data.extend_from_slice(&session_rent.to_le_bytes());
177181
create_ix_data.extend_from_slice(&(space as u64).to_le_bytes());
178182
create_ix_data.extend_from_slice(program_id.as_ref());
179183

‎program/src/processor/create_wallet.rs‎

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ use pinocchio::{
55
instruction::Seed,
66
program_error::ProgramError,
77
pubkey::{find_program_address, Pubkey},
8+
sysvars::rent::Rent,
89
ProgramResult,
910
};
1011

@@ -68,6 +69,7 @@ impl CreateWalletArgs {
6869
/// 3. `[writable]` Vault PDA: Derived from `["vault", wallet_pubkey]`.
6970
/// 4. `[writable]` Authority PDA: Derived from `["authority", wallet_pubkey, id_seed]`.
7071
/// 5. `[]` System Program.
72+
/// 6. `[]` Rent Sysvar.
7173
pub fn process(
7274
program_id: &Pubkey,
7375
accounts: &[AccountInfo],
@@ -108,6 +110,12 @@ pub fn process(
108110
let system_program = account_info_iter
109111
.next()
110112
.ok_or(ProgramError::NotEnoughAccountKeys)?;
113+
let rent_sysvar = account_info_iter
114+
.next()
115+
.ok_or(ProgramError::NotEnoughAccountKeys)?;
116+
117+
// Get rent from sysvar (fixes audit issue #5 - hardcoded rent calculations)
118+
let rent = Rent::from_account_info(rent_sysvar)?;
111119

112120
let (wallet_key, wallet_bump) = find_program_address(&[b"wallet", &args.user_seed], program_id);
113121
if !sol_assert_bytes_eq(wallet_pda.key().as_ref(), wallet_key.as_ref(), 32) {
@@ -131,13 +139,7 @@ pub fn process(
131139
// --- 1. Initialize Wallet Account ---
132140
// Calculate rent-exempt balance for fixed 8-byte wallet account layout.
133141
let wallet_space = 8;
134-
// 897840 + (space * 6960)
135-
let rent_base = 897840u64;
136-
let rent_per_byte = 6960u64;
137-
let wallet_rent = (wallet_space as u64)
138-
.checked_mul(rent_per_byte)
139-
.and_then(|val| val.checked_add(rent_base))
140-
.ok_or(ProgramError::ArithmeticOverflow)?;
142+
let wallet_rent = rent.minimum_balance(wallet_space);
141143

142144
// Use secure transfer-allocate-assign pattern to prevent DoS (Issue #4)
143145
let wallet_bump_arr = [wallet_bump];
@@ -182,12 +184,7 @@ pub fn process(
182184
};
183185

184186
let auth_space = header_size + variable_size;
185-
186-
// Rent calculation: 897840 + (space * 6960)
187-
let auth_rent = (auth_space as u64)
188-
.checked_mul(6960)
189-
.and_then(|val| val.checked_add(897840))
190-
.ok_or(ProgramError::ArithmeticOverflow)?;
187+
let auth_rent = rent.minimum_balance(auth_space);
191188

192189
// Use secure transfer-allocate-assign pattern to prevent DoS (Issue #4)
193190
let auth_bump_arr = [auth_bump];

‎sdk/src/instructions.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,7 @@ export async function createWalletInstruction(
127127
{ address: vaultPda, role: 1 }, // writable
128128
{ address: authorityPda, role: 1 }, // writable
129129
{ address: address("11111111111111111111111111111111"), role: 0 }, // system program
130+
{ address: address("SysvarRent111111111111111111111111111111111"), role: 0 }, // rent sysvar
130131
],
131132
data,
132133
};
@@ -260,6 +261,7 @@ export async function createSessionInstruction(
260261
{ address: authorizerPda, role: 1 },
261262
{ address: sessionPda, role: 1 },
262263
{ address: address("11111111111111111111111111111111"), role: 0 },
264+
{ address: address("SysvarRent111111111111111111111111111111111"), role: 0 }, // rent sysvar
263265
];
264266

265267
if (authorizerSigner) {

‎tests-e2e/Cargo.toml‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,7 @@ path = "src/main.rs"
99

1010
[dependencies]
1111
litesvm = "0.9.1"
12-
solana-pubkey = { version = "2.1", features = [
13-
"std",
14-
] } # Assuming 2.1 is compatible or latest
12+
solana-pubkey = { version = "2.1", features = ["std"] }
1513
solana-account = "3.4"
1614
solana-hash = "4.1"
1715
solana-keypair = "3.1"
@@ -21,8 +19,9 @@ solana-message = "3.0"
2119
solana-system-program = "3.1"
2220
solana-clock = "3.0"
2321
solana-signer = "3.0"
24-
solana-address = "2.1" # Check if litesvm uses 2.1 (yes, see output)
25-
solana-program = "2.1" # Contract uses this, keep it for constants?
22+
solana-address = "2.1"
23+
solana-program = "2.1"
24+
solana-sysvar = "3.0"
2625
anyhow = "1.0"
2726
rand = "0.8"
2827
p256 = { version = "0.13", features = ["ecdsa"] }

‎tests-e2e/src/scenarios/cross_wallet_attacks.rs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ use solana_message::Message;
66
use solana_pubkey::Pubkey;
77
use solana_signer::Signer;
88
use solana_system_program;
9+
use solana_sysvar;
910
use solana_transaction::Transaction;
1011

1112
pub fn run(ctx: &mut TestContext) -> Result<()> {
@@ -57,6 +58,7 @@ pub fn run(ctx: &mut TestContext) -> Result<()> {
5758
AccountMeta::new(vault_a.to_address(), false),
5859
AccountMeta::new(owner_a_auth.to_address(), false),
5960
AccountMeta::new_readonly(solana_system_program::id().to_address(), false),
61+
AccountMeta::new_readonly(solana_sysvar::rent::ID.to_address(), false),
6062
],
6163
data: data_a,
6264
};
@@ -87,6 +89,7 @@ pub fn run(ctx: &mut TestContext) -> Result<()> {
8789
AccountMeta::new(vault_b.to_address(), false),
8890
AccountMeta::new(owner_b_auth.to_address(), false),
8991
AccountMeta::new_readonly(solana_system_program::id().to_address(), false),
92+
AccountMeta::new_readonly(solana_sysvar::rent::ID.to_address(), false),
9093
],
9194
data: data_b,
9295
};

‎tests-e2e/src/scenarios/dos_attack.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ use solana_message::Message;
66
use solana_pubkey::Pubkey;
77
use solana_signer::Signer;
88
use solana_system_program;
9+
use solana_sysvar;
910
use solana_transaction::Transaction;
1011

1112
pub fn run(ctx: &mut TestContext) -> Result<()> {
@@ -79,6 +80,7 @@ pub fn run(ctx: &mut TestContext) -> Result<()> {
7980
AccountMeta::new(vault_pda.to_address(), false),
8081
AccountMeta::new(auth_pda.to_address(), false),
8182
AccountMeta::new_readonly(solana_system_program::id().to_address(), false),
83+
AccountMeta::new_readonly(solana_sysvar::rent::ID.to_address(), false),
8284
],
8385
data,
8486
};

‎tests-e2e/src/scenarios/failures.rs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ use solana_keypair::Keypair;
55
use solana_pubkey::Pubkey;
66
use solana_signer::Signer;
77
use solana_system_program;
8+
use solana_sysvar;
89
// use solana_transaction::Transaction; // Transaction usage needs refactor
910
use solana_message::Message;
1011
use solana_transaction::Transaction;
@@ -46,6 +47,7 @@ pub fn run(ctx: &mut TestContext) -> Result<()> {
4647
AccountMeta::new(Signer::pubkey(&owner_keypair).to_address(), true), // owner
4748
AccountMeta::new(Signer::pubkey(&ctx.payer).to_address(), true),
4849
AccountMeta::new_readonly(solana_system_program::id().to_address(), false),
50+
AccountMeta::new_readonly(solana_sysvar::rent::ID.to_address(), false),
4951
],
5052
data,
5153
};
@@ -262,6 +264,7 @@ pub fn run(ctx: &mut TestContext) -> Result<()> {
262264
AccountMeta::new(Signer::pubkey(&owner_keypair).to_address(), true),
263265
AccountMeta::new(Signer::pubkey(&ctx.payer).to_address(), true),
264266
AccountMeta::new_readonly(solana_system_program::id().to_address(), false),
267+
AccountMeta::new_readonly(solana_sysvar::rent::ID.to_address(), false),
265268
],
266269
data: [
267270
vec![1, 3], // AddAuthority(Session)

‎tests-e2e/src/scenarios/happy_path.rs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ use solana_program::hash::hash;
99
use solana_pubkey::Pubkey;
1010
use solana_signer::Signer;
1111
use solana_system_program;
12+
use solana_sysvar;
1213
use solana_transaction::Transaction;
1314

1415
pub fn run(ctx: &mut TestContext) -> Result<()> {
@@ -50,6 +51,7 @@ pub fn run(ctx: &mut TestContext) -> Result<()> {
5051
AccountMeta::new(vault_pda.to_address(), false),
5152
AccountMeta::new(owner_auth_pda.to_address(), false),
5253
AccountMeta::new_readonly(solana_system_program::id().to_address(), false),
54+
AccountMeta::new_readonly(solana_sysvar::rent::ID.to_address(), false),
5355
],
5456
data,
5557
};
@@ -204,6 +206,7 @@ pub fn run(ctx: &mut TestContext) -> Result<()> {
204206
AccountMeta::new_readonly(owner_auth_pda.to_address(), false),
205207
AccountMeta::new(session_pda.to_address(), false),
206208
AccountMeta::new_readonly(solana_system_program::id().to_address(), false),
209+
AccountMeta::new_readonly(solana_sysvar::rent::ID.to_address(), false),
207210
AccountMeta::new_readonly(Signer::pubkey(&owner_keypair).to_address(), true),
208211
],
209212
data: session_data,

‎tests-e2e/src/scenarios/mod.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
11
pub mod cross_wallet_attacks;
2+
pub mod dos_attack;
23
pub mod failures;
34
pub mod happy_path;

0 commit comments

Comments
 (0)