diff --git a/loan_manager/src/lib.rs b/loan_manager/src/lib.rs index a80aa45..7f0daef 100644 --- a/loan_manager/src/lib.rs +++ b/loan_manager/src/lib.rs @@ -1392,7 +1392,7 @@ impl LoanManager { } let loan_key = DataKey::Loan(loan_id); - let loan: Loan = env + let mut loan: Loan = env .storage() .persistent() .get(&loan_key) @@ -1419,27 +1419,32 @@ impl LoanManager { .storage() .instance() .get(&DataKey::Token) - .expect("token not set"); + .ok_or(LoanError::NotInitialized)?; let token_client = TokenClient::new(&env, &token); - token_client.transfer(&loan.borrower, &env.current_contract_address(), &amount); - - let loan_key = DataKey::Loan(loan_id); - let mut loan: Loan = env - .storage() - .persistent() - .get(&loan_key) - .expect("loan not found"); let updated_collateral = loan .collateral_amount .checked_add(amount) - .expect("collateral overflow"); + .ok_or(LoanError::AmountTooLarge)?; loan.collateral_amount = updated_collateral; env.storage().persistent().set(&loan_key, &loan); Self::bump_persistent_ttl(&env, &loan_key); + // CEI: commit the updated loan state before the external token transfer. + token_client.transfer(&loan.borrower, &env.current_contract_address(), &amount); + + // Re-validate after the external interaction with typed errors rather than panicking. + let stored_loan: Loan = env + .storage() + .persistent() + .get(&loan_key) + .ok_or(LoanError::LoanNotFound)?; + if stored_loan.status != LoanStatus::Approved { + return Err(LoanError::LoanNotActive); + } + env.events().publish( - (symbol_short!("ColDep"), loan_id, loan.borrower), + (symbol_short!("ColDep"), loan_id, stored_loan.borrower), updated_collateral, ); diff --git a/loan_manager/src/test.rs b/loan_manager/src/test.rs index 4cc0781..05dd20b 100644 --- a/loan_manager/src/test.rs +++ b/loan_manager/src/test.rs @@ -3,7 +3,40 @@ use lending_pool::{LendingPool, LendingPoolClient}; use remittance_nft::{RemittanceNFT, RemittanceNFTClient}; use soroban_sdk::testutils::Ledger as _; use soroban_sdk::token::{Client as TokenClient, StellarAssetClient}; -use soroban_sdk::{testutils::Address as _, Address, BytesN, Env, String}; +use soroban_sdk::{ + contract, contractimpl, testutils::Address as _, Address, BytesN, Env, String, Symbol, +}; + +#[contract] +pub struct MaliciousToken; + +#[contractimpl] +impl MaliciousToken { + pub fn set_attack_target(env: Env, manager: Address, loan_id: u32) { + env.storage() + .persistent() + .set(&Symbol::new(&env, "manager"), &manager); + env.storage() + .persistent() + .set(&Symbol::new(&env, "loan_id"), &loan_id); + } + + pub fn transfer(env: Env, _from: Address, _to: Address, _amount: i128) { + let manager: Address = env + .storage() + .persistent() + .get(&Symbol::new(&env, "manager")) + .unwrap(); + let loan_id: u32 = env + .storage() + .persistent() + .get(&Symbol::new(&env, "loan_id")) + .unwrap(); + env.as_contract(&manager, || { + env.storage().persistent().remove(&DataKey::Loan(loan_id)); + }); + } +} fn setup_test<'a>( env: &Env, @@ -1335,6 +1368,42 @@ fn test_deposit_collateral_and_auto_release_on_full_repayment() { ); } +#[test] +fn test_deposit_collateral_rejects_loan_removed_during_token_transfer() { + let env = Env::default(); + env.mock_all_auths_allowing_non_root_auth(); + + let (manager, nft_client, pool_client, token_id, _token_admin) = setup_test(&env); + let borrower = Address::generate(&env); + + let history_hash = BytesN::from_array(&env, &[0u8; 32]); + nft_client.mint( + &borrower, + &650, + &history_hash, + &String::from_str(&env, "ipfs://QmTest"), + &None, + ); + + let stellar_token = StellarAssetClient::new(&env, &token_id); + stellar_token.mint(&pool_client, &20_000); + stellar_token.mint(&borrower, &20_000); + + let loan_id = manager.request_loan(&borrower, &1_000, &17280); + manager.approve_loan(&loan_id); + + let malicious = env.register(MaliciousToken, ()); + let malicious_client = MaliciousTokenClient::new(&env, &malicious); + malicious_client.set_attack_target(&manager.address, &loan_id); + + env.as_contract(&manager.address, || { + env.storage().instance().set(&DataKey::Token, &malicious); + }); + + let result = manager.try_deposit_collateral(&loan_id, &300); + assert_eq!(result, Err(Ok(LoanError::LoanNotFound))); +} + #[test] fn test_collateral_is_seized_on_default() { let env = Env::default();