code-423n4 / 2022-01-xdefi-findings

0 stars 0 forks source link

Token IDs are not random #173

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

defsec

Vulnerability details

Impact

On the XDEFIDistribution contract, _generateNewTokenId function does not generate random numbers for the tokenIds. For the uniqueness of the token ids, the ids should be generated with pseudo random generator.

Proof of Concept

  1. Navigate to the following contract function.

https://github.com/XDeFi-tech/xdefi-distribution/blob/master/contracts/XDEFIDistribution.sol#L242

    function _generateNewTokenId(uint256 points_) internal view returns (uint256 tokenId_) {
        // Points is capped at 128 bits (max supply of XDEFI for 10 years locked), total supply of NFTs is capped at 128 bits.
        return (points_ << uint256(128)) + uint128(totalSupply() + 1);
    }

Tools Used

Manual Code Review

Recommended Mitigation Steps

If the value or desirability of the NFT's are different then it's important to use other ways to randomize, for example via a random oracle or a commit/reveal schema.

deluca-mike commented 2 years ago

Not only are tokenIds not require to be random, but in this specific case, we need the tokenIds to not be random, as they contain score information.