```returnIndex``` will revert if ```id``` is greater than or equal to 101

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/main/smart-contracts/XRandoms.sol#L15-L33 https://github.com/code-423n4/2023-10-nextgen/blob/main/smart-contracts/XRandoms.sol#L45-L47

Vulnerability details

Impact

The getWord function will revert if id is greater than or equal to 101. This is an undesirable behavior as the returnIndex function is marked as public meaning external users could use it and get their call unexpectedly reverted.

Proof of Concept

If a user inputs id = 105 into the returnIndex function below

    function returnIndex(uint256 id) public view returns (string memory) {
        return getWord(id);
    }

Then the function getWord below

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

will be called with id = 105 and will revert after trying to fetch wordsList[105 - 1] which does not exist as wordsList only contains 100 elements.

Tools Used

Visual Studio / Manual Review

Recommended Mitigation Steps

Use id % 100 instead of id as this will ensure the function getWord does not revert (as id % 100 will always be lesser than 100).

We should then replace

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

With

            return wordsList[id % 100];

This will not endanger the function’s behavior as it is used with a random number as input (getWord(randomNum) in the randomWord() function):

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

Alternatively, we should at least add a requirement to the getWord function like so

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

require(id <= 100, “id should be lower than 100”);        

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

And add comments to describe the function and its inputs to avoid users mistakenly using the function with id larger than 100.

Assessed type

Invalid Validation

[c4-pre-sort]

141345 marked the issue as insufficient quality report

[c4-pre-sort]

141345 marked the issue as remove high or low quality report

[c4-pre-sort]

141345 marked the issue as insufficient quality report

[141345]

expected behavior

[c4-judge]

alex-ppg marked the issue as duplicate of #1891

[c4-judge]

alex-ppg marked the issue as unsatisfactory: Invalid