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

Multiple self-profitable Registration-Accepted by the owner while other (not related to the user) users might get rejected to be registered.

nitnitish02

Root + Impact

Description

  • Normal behavior: register_pizza_lover should allow each user to be registered once.

  • Problem: It doesn’t check whether a user is already registered before calling get_random_slice, which internally calls table::add. If the user already exists, this aborts with a low-level table error.

  • Also, the registration should ideally be user-driven or trustless. But, here the owner has full control (centralisation) and there are chances that the owner might turn out to be mischievous and perform malpractice to take all the registrations of his own due to which allocations might be done by the owner to his self-controlled accounts.

// Root cause in the codebase with @> marks to highlight the relevant section
public entry fun register_pizza_lover(owner: &signer, user: address) acquires ModuleData, State {
let state = borrow_global_mut<State>(get_resource_address());
assert!(signer::address_of(owner) == state.owner, E_NOT_OWNER);
get_random_slice(user); // This is where the user is not being check for being registered already for allocation beforehand.
event::emit(PizzaLoverRegistered {
user: user, // since owner is accepting and confirming new users, it might turn out that a malicious owner might accept only self-profitable accounts while ignore other accounts - leading to one-sided profitability for the owner.
});
}
​

Risk

Likelihood:

  • Will frequently happen in practice due to user error or replayed transactions.

  • Always present — since all registrations are routed through the owner.

  • Malicious or careless owner behavior will directly impact fairness.

Impact:

  • Causes poor UX and may result in DoS if the owner’s automation retries registrations.

  • Owner can register fake addresses they control and drain the airdrop.

  • Legitimate users cannot self-register.

  • Centralized control undermines decentralization and fairness of the distribution.

Proof of Concept

What it proves: Because the owner is the sole registrar and there’s no pre-allocation budget check, the owner can register many addresses (possibly their own) and make the allocations towards his own addresses.

  • Because allocations are “random” (100–500), a small total_fund like 1_000 makes it very likely the first three claims deplete the pool; the victim’s claim_pizza_slice then reliably aborts with E_INSUFFICIENT_FUND.

  • This is a strong, behavior-level PoC for the centralization + over-allocation risk (the owner can create a registration order and enough addresses to starve others).

// Owner calls register_pizza_lover(owner_user1) once → success.
// Owner calls register_pizza_lover(owner_user2) once → success.
// Owner calls register_pizza_lover(legit_user) again → transaction aborts with table error.
​
// Owner registers many addresses → allocations exceed funding → later claims fail with E_INSUFFICIENT_FUND
#[test(
deployer = @pizza_drop,
a1 = @0xA1, a2 = @0xA2, a3 = @0xA3, victim = @0xBEEF,
framework = @0x1
)]
#[expected_failure(abort_code = E_INSUFFICIENT_FUND)]
fun test_owner_over_allocation_starves_victim(
deployer: &signer,
a1: &signer, a2: &signer, a3: &signer,
victim: &signer,
framework: &signer
) acquires State, ModuleData {
use aptos_framework::account;
use aptos_framework::timestamp;
use aptos_framework::aptos_coin;
​
// Setup basic environment
timestamp::set_time_has_started_for_testing(framework);
let (_burn, mint) = aptos_coin::initialize_for_test(framework);
​
// Create accounts
account::create_account_for_test(@pizza_drop);
account::create_account_for_test(signer::address_of(a1));
account::create_account_for_test(signer::address_of(a2));
account::create_account_for_test(signer::address_of(a3));
account::create_account_for_test(signer::address_of(victim));
​
// Initialize module and fund with a *small* pool to trigger insufficiency
init_module(deployer);
let total_fund = 1_000; // small pool
let coins = coin::mint<AptosCoin>(total_fund, &mint);
coin::register<AptosCoin>(deployer);
coin::deposit<AptosCoin>(@pizza_drop, coins);
fund_pizza_drop(deployer, total_fund);
​
// Owner registers arbitrary addresses (could be their own)
register_pizza_lover(deployer, signer::address_of(a1));
register_pizza_lover(deployer, signer::address_of(a2));
register_pizza_lover(deployer, signer::address_of(a3));
​
// Also register the victim (a legitimate user)
register_pizza_lover(deployer, signer::address_of(victim));
​
// Three addresses claim first; due to random 100–500 allocations,
// it's easy for the sum of the first three to exhaust the 1_000 pool.
claim_pizza_slice(a1);
claim_pizza_slice(a2);
claim_pizza_slice(a3);
​
// Now the victim attempts to claim, but pool is empty/insufficient → E_INSUFFICIENT_FUND
// This demonstrates how owner-driven registration & lack of pre-checks can starve others.
claim_pizza_slice(victim);
}

Recommended Mitigation

We need to remove the centralisation risk - no need for the user to be approved by the owner before being a part of the airdrop. Instead, any user could register for the airdrop and could get in without requiring any permission from the owner - to maintain fairness.

// to check for multiple addresses registered by the owner which might be his self-controlled ones while rejecting/ignoring all other addresses which do not belong to him and might cut-out on his profits.
- public entry fun register_pizza_lover(owner: &signer, user: address) acquires ModuleData, State {
- assert!(signer::address_of(owner) == state.owner, E_NOT_OWNER);
- get_random_slice(user);
- event::emit(PizzaLoverRegistered { user: user });
- }
​
+ public entry fun register_self(user: &signer) acquires ModuleData, State {
+ let user_addr = signer::address_of(user);
+ assert!(!table::contains(&state.users_claimed_amount, user_addr), E_ALREADY_REGISTERED);
+ get_random_slice(user_addr);
+ event::emit(PizzaLoverRegistered { user: user_addr });
+ }
// This shifts the trust model from “owner decides who gets in” to “any eligible user can enroll themselves,” eliminating censorship and reducing the blast radius of a malicious or careless owner.
​
+ add this code
Updates

Appeal created

bube Lead Judge 12 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.