Merge pull request #28 from lazor-kit/fix/audit-n1-n2-n3

fix(audit): N1 use auth_bump, N2 validate system_program, N3 check RP…

Author: onspeedhp

program/src/auth/secp256r1/mod.rs

@@ -69,6 +69,26 @@ impl Authenticator for Secp256r1Authenticator {
         // SAFETY: Pointer alignment checked above. size_of correct.
         let header = unsafe { &mut *(auth_data.as_mut_ptr() as *mut AuthorityAccountHeader) };
 
+        // Compute hash of user-provided RP ID and verify against stored hash (audit N3)
+        let stored_rp_id_hash = &auth_data[header_size..header_size + 32];
+        #[allow(unused_assignments)]
+        let mut computed_rp_id_hash = [0u8; 32];
+        #[cfg(target_os = "solana")]
+        unsafe {
+            let _res = pinocchio::syscalls::sol_sha256(
+                [rp_id].as_ptr() as *const u8,
+                1,
+                computed_rp_id_hash.as_mut_ptr(),
+            );
+        }
+        #[cfg(not(target_os = "solana"))]
+        {
+            computed_rp_id_hash = [0u8; 32];
+        }
+        if computed_rp_id_hash != stored_rp_id_hash {
+            return Err(AuthError::InvalidPubkey.into());
+        }
+
         #[allow(unused_assignments)]
         let mut hasher = [0u8; 32];
         #[cfg(target_os = "solana")]

program/src/processor/create_session.rs

@@ -97,6 +97,15 @@ pub fn process(
     // Get rent from sysvar (fixes audit issue #5 - hardcoded rent calculations)
     let rent = Rent::from_account_info(rent_sysvar)?;
 
+    // Validate system_program is the correct System Program (audit N2)
+    if !assertions::sol_assert_bytes_eq(
+        system_program.key().as_ref(),
+        &crate::utils::SYSTEM_PROGRAM_ID,
+        32,
+    ) {
+        return Err(ProgramError::IncorrectProgramId);
+    }
+
     if wallet_pda.owner() != program_id || authorizer_pda.owner() != program_id {
         return Err(ProgramError::IllegalOwner);
     }

program/src/processor/create_wallet.rs

@@ -4,7 +4,7 @@ use pinocchio::{
     account_info::AccountInfo,
     instruction::Seed,
     program_error::ProgramError,
-    pubkey::{find_program_address, Pubkey},
+    pubkey::{create_program_address, find_program_address, Pubkey},
     sysvars::rent::Rent,
     ProgramResult,
 };
@@ -117,6 +117,15 @@ pub fn process(
     // Get rent from sysvar (fixes audit issue #5 - hardcoded rent calculations)
     let rent = Rent::from_account_info(rent_sysvar)?;
 
+    // Validate system_program is the correct System Program (audit N2)
+    if !sol_assert_bytes_eq(
+        system_program.key().as_ref(),
+        &crate::utils::SYSTEM_PROGRAM_ID,
+        32,
+    ) {
+        return Err(ProgramError::IncorrectProgramId);
+    }
+
     let (wallet_key, wallet_bump) = find_program_address(&[b"wallet", &args.user_seed], program_id);
     if !sol_assert_bytes_eq(wallet_pda.key().as_ref(), wallet_key.as_ref(), 32) {
         return Err(ProgramError::InvalidSeeds);
@@ -129,8 +138,13 @@ pub fn process(
         return Err(ProgramError::InvalidSeeds);
     }
 
-    let (auth_key, auth_bump) =
-        find_program_address(&[b"authority", wallet_key.as_ref(), id_seed], program_id);
+    // Use client-provided auth_bump for efficiency (audit N1)
+    let auth_bump_arr = [args.auth_bump];
+    let auth_key = create_program_address(
+        &[b"authority", wallet_key.as_ref(), id_seed, &auth_bump_arr],
+        program_id,
+    )
+    .map_err(|_| ProgramError::InvalidSeeds)?;
     if !sol_assert_bytes_eq(auth_pda.key().as_ref(), auth_key.as_ref(), 32) {
         return Err(ProgramError::InvalidSeeds);
     }
@@ -187,7 +201,7 @@ pub fn process(
     let auth_rent = rent.minimum_balance(auth_space);
 
     // Use secure transfer-allocate-assign pattern to prevent DoS (Issue #4)
-    let auth_bump_arr = [auth_bump];
+    let auth_bump_arr = [args.auth_bump];
     let auth_seeds = [
         Seed::from(b"authority"),
         Seed::from(wallet_key.as_ref()),
@@ -211,7 +225,7 @@ pub fn process(
         discriminator: AccountDiscriminator::Authority as u8,
         authority_type: args.authority_type,
         role: 0,
-        bump: auth_bump,
+        bump: args.auth_bump,
         version: crate::state::CURRENT_ACCOUNT_VERSION,
         _padding: [0; 3],
         counter: 0,

program/src/processor/manage_authority.rs

@@ -5,6 +5,7 @@ use pinocchio::{
     instruction::Seed,
     program_error::ProgramError,
     pubkey::{find_program_address, Pubkey},
+    sysvars::rent::Rent,
     ProgramResult,
 };
 
@@ -133,6 +134,19 @@ pub fn process_add_authority(
         return Err(ProgramError::IllegalOwner);
     }
 
+    // Validate system_program is the correct System Program (audit N2)
+    if !sol_assert_bytes_eq(
+        system_program.key().as_ref(),
+        &crate::utils::SYSTEM_PROGRAM_ID,
+        32,
+    ) {
+        return Err(ProgramError::IncorrectProgramId);
+    }
+    let rent_sysvar_info = account_info_iter
+        .next()
+        .ok_or(ProgramError::NotEnoughAccountKeys)?;
+    let rent = Rent::from_account_info(rent_sysvar_info)?;
+
     // Check removed here, moved to type-specific logic
     // if !admin_auth_pda.is_writable() {
     //    return Err(ProgramError::InvalidAccountData);
@@ -194,11 +208,14 @@ pub fn process_add_authority(
     check_zero_data(new_auth_pda, ProgramError::AccountAlreadyInitialized)?;
 
     let header_size = std::mem::size_of::<AuthorityAccountHeader>();
-    let space = header_size + full_auth_data.len();
-    let rent = (space as u64)
-        .checked_mul(6960)
-        .and_then(|val| val.checked_add(897840))
-        .ok_or(ProgramError::ArithmeticOverflow)?;
+    // Secp256r1 needs extra 4 bytes for counter prefix
+    let variable_size = if args.authority_type == 1 {
+        4 + full_auth_data.len()
+    } else {
+        full_auth_data.len()
+    };
+    let space = header_size + variable_size;
+    let rent_lamports = rent.minimum_balance(space);
 
     // Use secure transfer-allocate-assign pattern to prevent DoS (Issue #4)
     let bump_arr = [bump];
@@ -214,7 +231,7 @@ pub fn process_add_authority(
         new_auth_pda,
         system_program,
         space,
-        rent,
+        rent_lamports,
         program_id,
         &seeds,
     )?;

program/src/processor/transfer_ownership.rs

@@ -112,6 +112,15 @@ pub fn process(
         return Err(ProgramError::IllegalOwner);
     }
 
+    // Validate system_program is the correct System Program (audit N2)
+    if !sol_assert_bytes_eq(
+        system_program.key().as_ref(),
+        &crate::utils::SYSTEM_PROGRAM_ID,
+        32,
+    ) {
+        return Err(ProgramError::IncorrectProgramId);
+    }
+
     if !current_owner.is_writable() {
         return Err(ProgramError::InvalidAccountData);
     }

program/src/utils.rs

@@ -7,6 +7,9 @@ use pinocchio::{
     ProgramResult,
 };
 
+/// System Program ID (11111111111111111111111111111111)
+pub const SYSTEM_PROGRAM_ID: [u8; 32] = [0u8; 32];
+
 /// Wrapper around the `sol_get_stack_height` syscall
 pub fn get_stack_height() -> u64 {
     #[cfg(target_os = "solana")]