Aptos Pizza Drop
Cyfrin
Beginner FriendlyGameFi
100 EXP
View results
Previous Next
Submission Details
Impact: high
Likelihood: high
Invalid

Reentrancy in fun claim_pizza_slice

sageceo

Root + Impact

Unprotected external calls in a state-changing function allow a malicious contract to re-enter the function before internal accounting is updated, draining the contract’s pizza slices balance.

Description

  • Normal behavior: A user deposits APT and later calls fun claim_pizza_slice() to receive their recorded balance.

  • Issue: fun claim_pizza_slice() performs an external call to user_addr before setting the contract’s balance to zero (and without a reentrancy lock). A malicious contract can use its fallback to call fun claim_pizza_slice() again during the first call, repeatedly claiminng more than its balance and draining the contract.

// Root cause in the codebase with @> marks to highlight the relevant section
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());
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);
};
//@Audit---------------------------------
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,
});

Risk

Likelihood:

  • Reason 1: Occurs whenever an attacker can deposit a minimal amount, then trigger fun claim_pizza_slice() from a contract with a fallback/receive that re-enters.

  • Reason 2

Impact:

  • Impact 1 Full drain of the contract’s pizza sice.

  • Impact 2 Protocol will lose users trust.

Proof of Concept

An Attacker deposite using callback funtion.
And call fun claim_pizza_slice.
Draining all the contract pizza slice

​

Recommended Mitigation

Update contract balance before making external calls.

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());
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);
};
​
// update balance before An external call.
​
- transfer_from_contract(user_addr, amount);
+ state.balance = state.balance - amount;
// Update balance
- state.balance = state.balance - amount;
+ transfer_from_contract(user_addr, amount);
table::add(&mut state.claimed_users, user_addr, true);
event::emit(PizzaClaimed {
user: user_addr,
amount: amount,
});
}
Updates

Appeal created

bube Lead Judge 12 days ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.