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

5 stars 3 forks source link

Word `Acai` have more chance to appear, The random numbers are not evenly distributed #670

Closed c4-submissions closed 11 months ago

c4-submissions commented 11 months ago

Lines of code

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

Vulnerability details

Impact

!not a duplicate issue of get random number from block.timestamp!

Word Acai have more chance to appear, The random numbers are not evenly distributed.

Proof of Concept

Function getWord is to get word by id, notice that when id= 0 or id= 1, it both will return word Acai.

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

        // array storing the words list
        string[100] memory wordsList = ["Acai", 
.......  "Watermelon"
];

        // returns a word based on index
        if (id==0) {
            return wordsList[id];
        } else {
 @>            return wordsList[id - 1];
        }
        }

The issue is when generating a random number to get word, suppose the random numbers are evenly distributed, id 0 and 1 will get the same result, which will makes the word Acai have more chance to appear disrupting the randomness.

poc:
forge test --mt test_getWorld -vv

pragma solidity ^0.8.13;

import "forge-std/Test.sol";

contract nextGenTest is Test {

    //forge test --mt test_getWorld -vv
    function test_getWorld() public {

          emit  log_named_string("getword0",getWord(0)) ;
          emit  log_named_string("getword1",getWord(1)) ;
          emit  log_named_string("getword2",getWord(2)) ;
          emit  log_named_string("getword99",getWord(99)) ;

    }

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

}

get log

Logs:
  getword0: Acai
  getword1: Acai
  getword2: Ackee
  getword99: Velvet Apple

Acai appear twice and last word Watermelon will never appear

Tools Used

manual

Recommended Mitigation Steps

consider modify the if logic


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

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)

c4-judge commented 10 months ago

alex-ppg marked the issue as grade-c