Beginner FriendlyGameFi
100 EXP
View results
Submission Details
Impact: medium
Likelihood: medium
Invalid

Balance Tracking Bypass in Pizza Drop Contract

Balance Tracking Bypass in Pizza Drop Contract

Description

  • The Pizza Drop contract is designed to allow users to claim APT tokens, with the contract maintaining sufficient funds to cover all claims.

  • The contract uses a state-tracked balance (state.balance) to validate if there were sufficient funds for claims. This state-tracked balance is updated when the fund_pizza_drop function is called, but this tracking could become out of sync with actual APT balance since direct transfers to the contract address bypass this tracking mechanism.

public entry fun claim_pizza_slice(user: &signer) acquires ModuleData, State {
// ...
@> let amount = *table::borrow(&state.users_claimed_amount, user_addr);
@> assert!(state.balance >= amount, E_INSUFFICIENT_FUND); // Uses tracked balance
// ...
}
public entry fun fund_pizza_drop(owner: &signer, amount: u64) {
// ...
coin::transfer<AptosCoin>(owner, resource_addr, amount);
@> state.balance = state.balance + amount; // Only updates on fund_pizza_drop
}

Risk

Likelihood:

  • When owner (or any user or contract) funds the contract via direct APT transfers (which is standard practice in Aptos)

  • The issue occurs any time funds arrive through means other than fund_pizza_drop

Impact:

  • Users cannot claim their allocated APT even when contract has sufficient funds

  • APT becomes locked in contract when tracked balance is lower than actual (unrecoverable without upgrading contracts)

Proof of Concept

The test below demonstrates how the balance tracking can be bypassed, leading to a state where users cannot claim their allocated APT even when the contract has sufficient funds. The test:

  1. Funds the contract with 50 APT through the proper fund_pizza_drop function

  2. Directly transfers 450 APT to the contract, bypassing the balance tracking

  3. Shows that while the contract actually holds 500 APT, it only tracks 50 APT

  4. Demonstrates that claims fail despite having 500 APT available, which should be enough to fund at least a single claim.

This scenario could happen in production if the owner, users or other contracts directly transfer APT to the resource account, a common practice in Aptos. The tracked balance would remain out of sync with the actual balance, potentially leading to permanently locked funds.

#[test]
fun test_balance_tracking_bypass(deployer: &signer, user: &signer) {
// Initial setup
init_module(deployer);
// Fund with 50 APT through proper channel
let proper_funding = 50;
fund_pizza_drop(deployer, proper_funding);
// Bypass tracking - send 450 APT directly
let bypass_amount = 450;
coin::transfer<AptosCoin>(deployer, get_resource_address(), bypass_amount);
// Demonstrate balance discrepancy
let tracked_balance = get_pizza_pool_balance();
let actual_balance = get_actual_apt_balance();
assert!(tracked_balance == 50); // Still shows only 50
assert!(actual_balance == 500); // Actually has 500
// Register user and attempt claim
register_pizza_lover(deployer, signer::address_of(user));
let claim_amount = get_claimed_amount(signer::address_of(user));
// Claim fails if amount > 50, despite having 500 available
if (claim_amount > 50) {
claim_pizza_slice(user); // Fails with E_INSUFFICIENT_FUND
}
}

Recommended Mitigation

Remove state balance tracking entirely and use actual APT balance for validation:

struct State has key {
users_claimed_amount: Table<address, u64>,
claimed_users: Table<address, bool>,
+ owner: address
- owner: address,
- balance: u64,
}
public entry fun claim_pizza_slice(user: &signer) acquires ModuleData, State {
// ...
let amount = *table::borrow(&state.users_claimed_amount, user_addr);
+ let current_balance = coin::balance<AptosCoin>(get_resource_address());
+ assert!(current_balance >= amount, E_INSUFFICIENT_FUND);
- assert!(state.balance >= amount, E_INSUFFICIENT_FUND);
// ...
}
public entry fun fund_pizza_drop(owner: &signer, amount: u64) {
// ...
coin::transfer<AptosCoin>(owner, resource_addr, amount);
- state.balance = state.balance + amount;
}
- #[view]
- public fun get_pizza_pool_balance(): u64 acquires ModuleData, State {
- let state = borrow_global<State>(get_resource_address());
- state.balance
- }

This solution:

  1. Uses a single source of truth (actual APT balance)

  2. Allows funding with direct transfers

  3. Prevents claims from failing when funds are available

  4. Simplifies the code by removing unnecessary state tracking

  5. Removes redundant view function get_pizza_pool_balance since actual balance can be queried using get_actual_apt_balance

Most importantly, this change ensures that APT sent directly to the resource account will not remain permanently locked if the module is published without upgrade capabilities. With the previous implementation, any APT sent directly would be inaccessible since the tracked balance would prevent claims, and without upgrade capabilities there would be no way to fix this state.

Updates

Appeal created

bube Lead Judge 9 days ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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