One Shot: Reloaded

First Flight #47
Beginner FriendlyNFT
100 EXP
Submission Details
Impact: medium
Likelihood: medium

[M-02] Missing Access Control on Internal Functions

Author Revealed upon completion

Root + Impact

Description

The protocol's public(friend) functions lack proper access control validation, allowing unauthorized state modifications between modules. Functions like write_stats(), transfer_record_only(), and increment_wins() can be called without validating the caller context, creating potential race conditions and unauthorized state changes.

These internal functions can be invoked by any friend module without verifying the operation type or authorization level, leading to potential state corruption and security bypasses.

// In one_shot.move
@> public(friend) fun write_stats(
@> token_id: address,
@> weak_knees: bool,
@> heavy_arms: bool,
@> spaghetti_sweater: bool,
@> calm_and_ready: bool,
@> battles_won: u64
@> ) acquires RapperStats {
@> let stats_res = borrow_global_mut<RapperStats>(@battle_addr);
@> let s = table::borrow_mut(&mut stats_res.stats, token_id);
@> // No validation of caller context or operation type
@> s.weak_knees = weak_knees;
@> s.heavy_arms = heavy_arms;
@> s.spaghetti_sweater = spaghetti_sweater;
@> s.calm_and_ready = calm_and_ready;
@> s.battles_won = battles_won;
@> }

Risk

Likelihood:

  • Any friend module can call these functions without restriction

  • The lack of context validation allows unauthorized state modifications

  • Race conditions can occur between staking and battling operations

Impact:

  • Unauthorized modification of NFT attributes

  • State corruption between different protocol operations

  • Security bypasses through direct function calls

Proof of Concept

This PoC demonstrates how unauthorized state modification can occur:

// Attack scenario demonstrating unauthorized access
let malicious_module = /* hypothetical friend module */;
let target_token = @0x1234567890abcdef;
// Unauthorized state modification attack
// This bypasses normal progression rules
one_shot::write_stats(
target_token,
false, // weak_knees (should require staking)
false, // heavy_arms (should require staking)
false, // spaghetti_sweater (should require staking)
true, // calm_and_ready (should require 4+ days)
100 // battles_won (should require actual battles)
);
// Result: Stats are artificially enhanced without going through normal progression
// This creates unfair advantages and breaks the game's progression system
// Another attack: Race condition between staking and battling
// streets::write_stats called simultaneously with rap_battle::increment_wins
// could lead to inconsistent state

Recommended Mitigation

The mitigation implements role-based access control with operation context validation:

+ struct CallContext has store {
+ caller_module: address,
+ operation_type: u8, // 1=stake, 2=battle, 3=admin
+ authorized: bool,
+ }
+
+ const E_INVALID_CONTEXT: u64 = 10;
+ const E_UNAUTHORIZED_CALLER: u64 = 11;
+
public(friend) fun write_stats(
+ context: &CallContext,
token_id: address,
weak_knees: bool,
heavy_arms: bool,
spaghetti_sweater: bool,
calm_and_ready: bool,
battles_won: u64
) acquires RapperStats {
+ // Validate caller context
+ assert!(context.authorized == true, E_UNAUTHORIZED_CALLER);
+
+ // Validate operation type
+ let valid_operations = vector::singleton<u8>(1); // Only staking can modify these
+ vector::push_back(&mut valid_operations, 2); // Only battles can modify wins
+ assert!(vector::contains(&valid_operations, &context.operation_type), E_INVALID_CONTEXT);
+
+ // Validate progression rules
+ let stats_res = borrow_global_mut<RapperStats>(@battle_addr);
+ let s = table::borrow_mut(&mut stats_res.stats, token_id);
+
+ // Enforce progression rules based on operation type
+ if (context.operation_type == 1) { // Staking operation
+ // Allow modification of vices and virtues
+ s.weak_knees = weak_knees;
+ s.heavy_arms = heavy_arms;
+ s.spaghetti_sweater = spaghetti_sweater;
+ s.calm_and_ready = calm_and_ready;
+ } else if (context.operation_type == 2) { // Battle operation
+ // Only allow incrementing battles_won
+ s.battles_won = battles_won;
+ };
}

This mitigation adds comprehensive access control with operation-type validation and progression rule enforcement, preventing unauthorized state modifications.

Support

FAQs

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