Potential Reentrancy Risk
Description
The pizza_drop::transfer_from_contract function calls coin::transfer which could potentially be reentrant if the receiver is a malicious contract that implements a fallback function.
fun transfer_from_contract(to: address, amount: u64) acquires ModuleData {
    let module_data = borrow_global<ModuleData>(@pizza_drop);
    let resource_signer = account::create_signer_with_capability(&module_data.signer_cap);
    
    // @? REENTRANCY RISK: coin::transfer can potentially trigger
    
    coin::transfer<AptosCoin>(&resource_signer, to, amount);
}
Risk
Likelihood: Medium
Impact:
Proof of Concept
- A malicious user can create a contract that implements a fallback function. 
- When coin::transfer is called in transfer_from_contract, the malicious contract's fallback function gets executed In this fallback function, the malicious contract could attempt to call other functions in the PizzaDrop contract. 
- If any state changes haven't been committed yet, the malicious contract might be able to manipulate the contract's state 
module malicious::reentrancy_attacker {
    use pizza_drop::airdrop;
    use aptos_framework::coin;
    use aptos_framework::aptos_coin::AptosCoin;
    
    struct AttackState has key {
        claimed: bool,
        target_contract: address
    }
    
    public entry fun attack_pizza_drop(target: address, user: &signer) {
        
        
        
        // Set up attack state
        move_to(user, AttackState {
            claimed: false,
            target_contract: target
        })
        
        
        // airdrop::claim_pizza_slice(user)
    }
    
    // This function gets called when receiving APT
    public fun receive_coins(sender: address, amount: u64) {
        let attack_state = borrow_global_mut<AttackState>(sender);
        
        
        // the first call is still executing
        if (!attack_state.claimed) {
            attack_state.claimed = true
            // airdrop::claim_pizza_slice(...)
        }
    }
}
Recommended Mitigation
- We need to follow the check-effects-interactions(CEI) pattern by updating all state variables before making external calls. 
// In claim_pizza_slice function (lines 70-92)
 
public entry fun claim_pizza_slice(user: &signer) acquires ModuleData, State {
    let user_addr = signer::address_of(user);
    let state = borrow_global_mut<State>(get_resource_address());
    
    // Checks first
    assert!(table::contains(&state.users_claimed_amount, user_addr), E_NOT_REGISTERED);
    assert!(!table::contains(&state.claimed_users, user_addr), E_ALREADY_CLAIMED);
    
    let amount = *table::borrow(&state.users_claimed_amount, user_addr);
    
    // Check if contract has sufficient balance
    assert!(state.balance >= amount, E_INSUFFICIENT_FUND);
    
    // Effects (state changes) - BEFORE external calls
+   // Mark as claimed BEFORE external call to prevent reentrancy
+   table::add(&mut state.claimed_users, user_addr, true);
+   
+   // Update balance BEFORE external call
+   state.balance = state.balance - amount;
+   
+   // Register user to receive APT if not already registered
+   if (!coin::is_account_registered<AptosCoin>(user_addr)) {
+       coin::register<AptosCoin>(user);
+   };
+   
+   // Emit event BEFORE external call
+   event::emit(PizzaClaimed {
+       user: user_addr,
+       amount: amount,
+   });
    
-   // Register user to receive APT if not already registered
-   if (!coin::is_account_registered<AptosCoin>(user_addr)) {
-       coin::register<AptosCoin>(user);
-   };
-   
-   transfer_from_contract(user_addr, amount);
-   
-   // Update balance
-   state.balance = state.balance - amount;
-   
-   table::add(&mut state.claimed_users, user_addr, true);
-   
-   event::emit(PizzaClaimed {
-       user: user_addr,
-       amount: amount,
-   });
+   // Interactions (external calls) - LAST
+   transfer_from_contract(user_addr, amount);
}
- We can use reentrancy guard to the State struct. 
struct State has key {
    users_claimed_amount: Table<address, u64>,
    claimed_users: Table<address, bool>,
    owner: address,
    balance: u64,
+   locked: bool,  // Reentrancy guard
}
Also add this in fun pizza_drop::claim_pizza_slice function
public entry fun claim_pizza_slice(user: &signer) acquires ModuleData, State {
+   let state = borrow_global_mut<State>(get_resource_address());
+   
+   // @? REENTRANCY PROTECTION: Check if function is already executing
+   assert!(!state.locked, 100); // E_REENTRANCY
+   state.locked = true;
    
   let user_addr = signer::address_of(user);
        let state = borrow_global_mut<State>(get_resource_address());
        
        assert!(table::contains(&state.users_claimed_amount, user_addr), E_NOT_REGISTERED);
        assert!(!table::contains(&state.claimed_users, user_addr), E_ALREADY_CLAIMED);
        
        let amount = *table::borrow(&state.users_claimed_amount, user_addr);
        
        // Check if contract has sufficient balance
        assert!(state.balance >= amount, E_INSUFFICIENT_FUND);
        
        // Register user to receive APT if not already registered
        if (!coin::is_account_registered<AptosCoin>(user_addr)) {
            coin::register<AptosCoin>(user);
        };
        
        transfer_from_contract(user_addr, amount);
        
        // Update balance
        state.balance = state.balance - amount;
        
        table::add(&mut state.claimed_users, user_addr, true);
        
        event::emit(PizzaClaimed {
            user: user_addr,
            amount: amount,
        });
    
+   // Reset lock at the end
+   state.locked = false;
}