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

Getter Function Violates State Handling Convention

0xrektified

Description

The get_random_slice function violates standard coding conventions by modifying state despite its name suggesting it should be a read-only getter function.
Getter functions should return values without side effects, but this function directly modifies the users_claimed_amount table.

Root Cause

The function is named as a getter (get_random_slice) but performs state modification operations instead of just returning a computed value. This breaks the separation of concerns and violates the expected behavior of getter functions.

entry fun get_random_slice(user_addr: address) acquires ModuleData, State {
let state = borrow_global_mut<State>(get_resource_address());
let time = timestamp::now_microseconds();
let random_val = time % 401;
let random_amount = 100 + random_val;
table::add(&mut state.users_claimed_amount, user_addr, random_amount); // Modifies state!
}
​
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); // Expects state modification as side effect
// ...
}

Risk

Likelihood: High - Occurs every time the function is called

Impact: Low - Improper state handling that doesn't follow coding conventions

Impact

  • Incorrect state handling: Function behavior contradicts its naming and expected functionality

  • Unexpected side effects: Developers calling "getter" functions may not expect state modifications

  • Protocol logic confusion: State changes hidden in getter functions can lead to integration errors

  • Function responsibility violation: Single function handles both computation and state modification incorrectly

Proof of Concept

Current problematic implementation:

// Function name suggests "getting" but actually "sets" state
entry fun get_random_slice(user_addr: address) acquires ModuleData, State {
let state = borrow_global_mut<State>(get_resource_address());
let time = timestamp::now_microseconds();
let random_val = time % 401;
let random_amount = 100 + random_val;
table::add(&mut state.users_claimed_amount, user_addr, random_amount); // Side effect!
}
​
public entry fun register_pizza_lover(owner: &signer, user: address) {
// ...
get_random_slice(user); // Caller expects state modification as side effect
// ...
}

Issues identified:

  • Function named get_* but performs table::add() operation affecting protocol state

  • Violates expected behavior for getter functions, potentially causing integration issues

  • Mixed responsibilities create incorrect state handling patterns

Recommended Mitigation

Separate the random computation from state modification to follow proper conventions:

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 value (no state modification)
+ let random_amount = get_random_slice();
+
+ // Explicitly handle state modification in caller
+ table::add(&mut state.users_claimed_amount, user, random_amount);
- get_random_slice(user); // Previously modified state as side effect
event::emit(PizzaLoverRegistered { user: user });
}
​
// Proper getter function - read-only, no side effects
+ #[view]
+ fun get_random_slice(): u64 {
+ let time = timestamp::now_microseconds();
+ let random_val = time % 401;
+ 100 + random_val
+ }

Benefits of proper separation:

  • Correct state handling: Function behavior matches its name and purpose

  • Protocol integrity: State modifications are explicit and predictable

  • Integration safety: External developers won't encounter unexpected state changes

  • Function correctness: Each function has a single, clear responsibility

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.