c4-submissions commented 11 months ago

Lines of code

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

Vulnerability details

Impact

XRandoms.getWord is expected to return a word with a index between [0~99], including 0, and 99. But according to current implementation, the function will not work as expect.

Proof of Concept

Before calling randomPool.getWord, function randomPool.randomWord will generate a number between [0, 99], include 0 and 99, then calling getWord with the generated number.

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

Based on randomWord, the parameter for getWord will be [0, 99], and in randomPool.getWord, it supposed that getWord should return a word from wordsList with a index between [0,99], but the code will return the first word Acai twice when id = 0 and id = 1, and the last word Watermelon will never be returned because of XRandoms.sol#L31

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

Following is little POC, run the following code in chisel with traces enabled, we can see that

contract A {
    event Log(string);
    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"];

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

    function tr() public {
        string memory word;
        for(uint i; i < 100; i ++){
            word = getWord(i);
            emit Log(word);
        }
    }
}

Run the above code in chisel, and we can see that Acai is returned twice, and Watermelon is never returned ➜ A a = new A() ➜ !t Enabled traces! ➜ a.tr() ➜ a.tr(); Traces: [4082828] 0xBd770416a3345F91E4B34576cb804a576fa48EB1::run() ├─ [1325776] → new @0xf4D9599aFd90B5038b18e3B551Bc21a97ed21c37 │ └─ ← 6622 bytes of code ├─ [2723146] 0xf4D9599aFd90B5038b18e3B551Bc21a97ed21c37::tr() │ ├─ emit Log(: Acai) │ ├─ emit Log(: Acai) │ ├─ emit Log(: Ackee) │ ├─ emit Log(: Apple) │ ├─ emit Log(: Apricot) │ ├─ emit Log(: Avocado) │ ├─ emit Log(: Babaco) │ ├─ emit Log(: Banana) │ ├─ emit Log(: Bilberry) │ ├─ emit Log(: Blackberry) │ ├─ emit Log(: Blackcurrant) │ ├─ emit Log(: Blood Orange) │ ├─ emit Log(: Blueberry) │ ├─ emit Log(: Boysenberry) │ ├─ emit Log(: Breadfruit) │ ├─ emit Log(: Brush Cherry) │ ├─ emit Log(: Canary Melon) │ ├─ emit Log(: Cantaloupe) │ ├─ emit Log(: Carambola) │ ├─ emit Log(: Casaba Melon) │ ├─ emit Log(: Cherimoya) │ ├─ emit Log(: Cherry) │ ├─ emit Log(: Clementine) │ ├─ emit Log(: Cloudberry) │ ├─ emit Log(: Coconut) │ ├─ emit Log(: Cranberry) │ ├─ emit Log(: Crenshaw Melon) │ ├─ emit Log(: Cucumber) │ ├─ emit Log(: Currant) │ ├─ emit Log(: Curry Berry) │ ├─ emit Log(: Custard Apple) │ ├─ emit Log(: Damson Plum) │ ├─ emit Log(: Date) │ ├─ emit Log(: Dragonfruit) │ ├─ emit Log(: Durian) │ ├─ emit Log(: Eggplant) │ ├─ emit Log(: Elderberry) │ ├─ emit Log(: Feijoa) │ ├─ emit Log(: Finger Lime) │ ├─ emit Log(: Fig) │ ├─ emit Log(: Gooseberry) │ ├─ emit Log(: Grapes) │ ├─ emit Log(: Grapefruit) │ ├─ emit Log(: Guava) │ ├─ emit Log(: Honeydew Melon) │ ├─ emit Log(: Huckleberry) │ ├─ emit Log(: Italian Prune Plum) │ ├─ emit Log(: Jackfruit) │ ├─ emit Log(: Java Plum) │ ├─ emit Log(: Jujube) │ ├─ emit Log(: Kaffir Lime) │ ├─ emit Log(: Kiwi) │ ├─ emit Log(: Kumquat) │ ├─ emit Log(: Lemon) │ ├─ emit Log(: Lime) │ ├─ emit Log(: Loganberry) │ ├─ emit Log(: Longan) │ ├─ emit Log(: Loquat) │ ├─ emit Log(: Lychee) │ ├─ emit Log(: Mammee) │ ├─ emit Log(: Mandarin) │ ├─ emit Log(: Mango) │ ├─ emit Log(: Mangosteen) │ ├─ emit Log(: Mulberry) │ ├─ emit Log(: Nance) │ ├─ emit Log(: Nectarine) │ ├─ emit Log(: Noni) │ ├─ emit Log(: Olive) │ ├─ emit Log(: Orange) │ ├─ emit Log(: Papaya) │ ├─ emit Log(: Passion fruit) │ ├─ emit Log(: Pawpaw) │ ├─ emit Log(: Peach) │ ├─ emit Log(: Pear) │ ├─ emit Log(: Persimmon) │ ├─ emit Log(: Pineapple) │ ├─ emit Log(: Plantain) │ ├─ emit Log(: Plum) │ ├─ emit Log(: Pomegranate) │ ├─ emit Log(: Pomelo) │ ├─ emit Log(: Prickly Pear) │ ├─ emit Log(: Pulasan) │ ├─ emit Log(: Quine) │ ├─ emit Log(: Rambutan) │ ├─ emit Log(: Raspberries) │ ├─ emit Log(: Rhubarb) │ ├─ emit Log(: Rose Apple) │ ├─ emit Log(: Sapodilla) │ ├─ emit Log(: Satsuma) │ ├─ emit Log(: Soursop) │ ├─ emit Log(: Star Apple) │ ├─ emit Log(: Star Fruit) │ ├─ emit Log(: Strawberry) │ ├─ emit Log(: Sugar Apple) │ ├─ emit Log(: Tamarillo) │ ├─ emit Log(: Tamarind) │ ├─ emit Log(: Tangelo) │ ├─ emit Log(: Tangerine) │ ├─ emit Log(: Ugli) │ ├─ emit Log(: Velvet Apple) │ └─ ← () └─ ← ()

Tools Used

VIM

Recommended Mitigation Steps

--- a/hardhat/smart-contracts/XRandoms.sol
+++ b/hardhat/smart-contracts/XRandoms.sol
@@ -25,11 +25,7 @@ contract randomPool {
         "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];
-        }
+        return wordsList[id];
         }

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)