code-423n4 / 2023-10-nextgen-findings

5 stars 3 forks source link

The `randomWord` method of `randomPool` contract have a wrong random distribution. #220

Closed c4-submissions closed 10 months ago

c4-submissions commented 11 months ago

Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/hardhat/smart-contracts/XRandoms.sol#L28

Vulnerability details

The randomWord method of randomPool contract have a wrong random distribution.

Vulnerable Code

contract randomPool {

    function getWord(uint256 id) private pure returns (string memory) {

        // array storing the words list
        string[100] memory wordsList = ["Acai", "Ackee", "Apple", "Apricot", "Avocado", "Babaco", "Banana", "Bilberry", "Blackberry", "Blackcurrant", "Blood Orange",
                    "Blueberry", "Boysenberry", "Breadfruit", "Brush Cherry", "Canary Melon", "Cantaloupe", "Carambola", "Casaba Melon", "Cherimoya", "Cherry", "Clementine",
                    "Cloudberry", "Coconut", "Cranberry", "Crenshaw Melon", "Cucumber", "Currant", "Curry Berry", "Custard Apple", "Damson Plum", "Date", "Dragonfruit", "Durian",
                    "Eggplant", "Elderberry", "Feijoa", "Finger Lime", "Fig", "Gooseberry", "Grapes", "Grapefruit", "Guava", "Honeydew Melon", "Huckleberry", "Italian Prune Plum",
                    "Jackfruit", "Java Plum", "Jujube", "Kaffir Lime", "Kiwi", "Kumquat", "Lemon", "Lime", "Loganberry", "Longan", "Loquat", "Lychee", "Mammee", "Mandarin", "Mango",
                    "Mangosteen", "Mulberry", "Nance", "Nectarine", "Noni", "Olive", "Orange", "Papaya", "Passion fruit", "Pawpaw", "Peach", "Pear", "Persimmon", "Pineapple",
                    "Plantain", "Plum", "Pomegranate", "Pomelo", "Prickly Pear", "Pulasan", "Quine", "Rambutan", "Raspberries", "Rhubarb", "Rose Apple", "Sapodilla", "Satsuma",
                    "Soursop", "Star Apple", "Star Fruit", "Strawberry", "Sugar Apple", "Tamarillo", "Tamarind", "Tangelo", "Tangerine", "Ugli", "Velvet Apple", "Watermelon"];

        // returns a word based on index
        if (id==0) {
            return wordsList[id];
        } else {
            return wordsList[id - 1];
        }
    }
    ...
    function randomWord() public view returns (string memory) {
        uint256 randomNum = uint(keccak256(abi.encodePacked(block.prevrandao, blockhash(block.number - 1), block.timestamp))) % 100;
        return getWord(randomNum);
    }
    ...
}

Link

Explanation

This function is used in randomWord to obtain a random word of the array wordsList. The problem is that the random number with a modulus of 100. So when entering getWord, the if condition will make a wrong random distribution of the wordList where the first word has twice the probability of being selected than the others and the last word can't be selected.

POC

from hashlib import sha3_256
from random import randbytes

def get_rand():
    return int(sha3_256(randbytes(32)).hexdigest(), 16) % 100

if __name__ == "__main__":
    rand_distribution = [0] * 100
    for _ in range(1_000_000):
        rand = get_rand()
        if rand == 0:
            rand_distribution[rand] += 1
        else:
            rand_distribution[rand - 1] += 1
    print(rand_distribution)

Remediation

Remove the if condition in getWord and return directly the word, it will also cost less gas.

contract randomPool {

    function getWord(uint256 id) private pure returns (string memory) {

        // array storing the words list
        string[100] memory wordsList = ["Acai", "Ackee", "Apple", "Apricot", "Avocado", "Babaco", "Banana", "Bilberry", "Blackberry", "Blackcurrant", "Blood Orange",
                    "Blueberry", "Boysenberry", "Breadfruit", "Brush Cherry", "Canary Melon", "Cantaloupe", "Carambola", "Casaba Melon", "Cherimoya", "Cherry", "Clementine",
                    "Cloudberry", "Coconut", "Cranberry", "Crenshaw Melon", "Cucumber", "Currant", "Curry Berry", "Custard Apple", "Damson Plum", "Date", "Dragonfruit", "Durian",
                    "Eggplant", "Elderberry", "Feijoa", "Finger Lime", "Fig", "Gooseberry", "Grapes", "Grapefruit", "Guava", "Honeydew Melon", "Huckleberry", "Italian Prune Plum",
                    "Jackfruit", "Java Plum", "Jujube", "Kaffir Lime", "Kiwi", "Kumquat", "Lemon", "Lime", "Loganberry", "Longan", "Loquat", "Lychee", "Mammee", "Mandarin", "Mango",
                    "Mangosteen", "Mulberry", "Nance", "Nectarine", "Noni", "Olive", "Orange", "Papaya", "Passion fruit", "Pawpaw", "Peach", "Pear", "Persimmon", "Pineapple",
                    "Plantain", "Plum", "Pomegranate", "Pomelo", "Prickly Pear", "Pulasan", "Quine", "Rambutan", "Raspberries", "Rhubarb", "Rose Apple", "Sapodilla", "Satsuma",
                    "Soursop", "Star Apple", "Star Fruit", "Strawberry", "Sugar Apple", "Tamarillo", "Tamarind", "Tangelo", "Tangerine", "Ugli", "Velvet Apple", "Watermelon"];

        // returns a word based on index
        return wordsList[id];
    }
    ...
}
from hashlib import sha3_256
from random import randbytes

def get_rand():
    return int(sha3_256(randbytes(32)).hexdigest(), 16) % 100

if __name__ == "__main__":
    rand_distribution = [0] * 100
    for _ in range(1_000_000):
        rand_distribution[get_rand()] += 1
    print(rand_distribution)

Assessed type

Other

c4-pre-sort commented 10 months ago

141345 marked the issue as duplicate of #508

c4-judge commented 10 months ago

alex-ppg changed the severity to QA (Quality Assurance)

c4-judge commented 10 months ago

alex-ppg marked the issue as grade-c