c4-submissions commented 11 months ago

Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/main/smart-contracts/XRandoms.sol#L40-L43

Vulnerability details

Impact

In the randomWord() function, the number

uint256 randomNum = uint(keccak256(abi.encodePacked(block.prevrandao, blockhash(block.number - 1), block.timestamp))) % 100;

is passed to the getWord function, which will return the element of the wordsList corresponding to the id passed as parameter. However the first element of the list (”Acai”) will be returned for 2 distinct values of id respectively 0 and and 1. This endangers the randomness of the function and could lead to potential exploits.

Proof of Concept

Let’s have a look at the getWord function

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];
        }
}

For id == 0, this function will return wordsList[id] i.e. wordsList[0] i.e. ”Acai”. For id==1, this function will return wordsList[id - 1] i.e. wordsList[0] i.e. ”Acai”. This means elements from wordsList don’t have the same probability of being picked, which is bad practice for a randomizing function. This behavior should be fixed.

Tools Used

Visual Studio / Manual Review

Recommended Mitigation Steps

In the getWord function, we should replace

if (id==0) {
     return wordsList[id];
} else {
     return wordsList[id - 1];
 }

With

return wordsList[id % 100];

This will ensure all words in wordsList have an equal probability of being picked.

Alternatively, the returnIndex should at least use a require statement and a comment to describe function’s behavior and inputs to avoid confusion of the user. Such statement would look like

require(id <= 100, “id should be lesser or equal to 100”);

Assessed type

Other

c4-pre-sort commented 11 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)