c4-submissions commented 11 months ago

Lines of code

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

Vulnerability details

Impact

The last element of the string[100] variable wordsList can not be reached.

Proof of Concept

    function randomNumber() public view returns (uint256){
        uint256 randomNum = uint(keccak256(abi.encodePacked(block.prevrandao, blockhash(block.number - 1), block.timestamp))) % 1000;
        return randomNum;
    }

The above function first calculates a random number based on block.number and block.timestamp, and then takes the modulo of this value by 100 。 The range of the result value will be from 0 to 99. The result value will be passed to the getWord function as a parameter id.

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

If the value is 0 return the first element of the wordsList. However if the value if 99 getWord return the index 98(99-1) the penultimate element of wordsList

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;

import {Test, console2} from "forge-std/Test.sol";

contract XRandomTest is Test {
    function setUp() public {}

    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 compareStrings(string memory a, string memory b) public pure returns (bool) {
        return (keccak256(abi.encodePacked((a))) == keccak256(abi.encodePacked((b))));
    }

    function testGetWorld() public {
        assert(compareStrings(getWord(0),"Acai"));//0 == first element.
        assert(compareStrings(getWord(99), "Velvet Apple"));//99 == penultimate element.
        assert(compareStrings(getWord(100), "Watermelon"));//100 == last element.
    }

}

From above test we can see only id == 100 we can get the last value of wordsList

Tools Used

manure review

Recommended Mitigation Steps

the range of randomNum if from 0-99 and the index of wordsList is also from 0-99 so id-1 is not necessary in function getWord

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

Assessed type

Math

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)