Lines of code
<https://github.com/code-423n4/2023-03-wenwin/blob/91b89482aaedf8b8feb73c771d11c257eed997e8/src/LotterySetup.sol#L120&gt;

Vulnerability details

Impact

If the rewardToken has less than 16 decimals, users will not be able to claim the correct value of their prize.

> Try not to think it will be DAI only. We will probably use DAI for the first product, but think more in general approach
@0xluckydev#5064

It is not the case with DAI (since it has 18 decimals), but it would be an issue with USDC and USDT for example.

Proof of Concept

In the TestToken.sol, add the following function to change the number of decimals easily:

    function decimals() public pure override returns (uint8) {
        return 18;
    }

Run forge test --match-test testFixedReward -vvvv with different values of decimals() like 18 (should pass) and 6 (the test will not pass).

In the function packFixedRewards (LotterySetup.sol), uint8 reward is overflowed when a low decimal token is used

    function packFixedRewards(uint256[] memory rewards) private view returns (uint256 packed) {
        if (rewards.length != (selectionSize) || rewards[0] != 0) {
            revert InvalidFixedRewardSetup();
        }
        uint256 divisor = 10 ** (IERC20Metadata(address(rewardToken)).decimals() - 1);
        for (uint8 winTier = 1; winTier < selectionSize; ++winTier) {
            uint16 reward = uint16(rewards[winTier] / divisor);
            if ((rewards[winTier] % divisor) != 0) {
                revert InvalidFixedRewardSetup();
            }
            packed |= uint256(reward) << (winTier * 16);
        }
    }

In order to reproduce the following outputs, you will need to update LotterySetup.sol :

• Add the import “forge-std/Test.sol”;

• Inherit from Test so we can have console.log contract LotterySetup is ILotterySetup, Test {

• Update the packFixedRewards function (it will just add some console.log statements)

  function packFixedRewards(uint256[] memory rewards) private view returns (uint256 packed) {
      if (rewards.length != (selectionSize) || rewards[0] != 0) {
          revert InvalidFixedRewardSetup();
      }
      uint256 divisor = 10 ** (IERC20Metadata(address(rewardToken)).decimals() - 1);
      for (uint8 winTier = 1; winTier < selectionSize; ++winTier) {
          uint16 reward = uint16(rewards[winTier] / divisor);
          console.log("rewards[winTier]  %s", rewards[winTier]);
  
          console.log("divisor %s", divisor);
  
          console.log("max U16 %s", type(uint16).max);
          console.log("reward %s", reward);
          console.log("rewards[winTier] %s", rewards[winTier]);
  
          if ((rewards[winTier] % divisor) != 0) {
              revert InvalidFixedRewardSetup();
          }
          packed |= uint256(reward) << (winTier * 16);
          console.log("=============");
      }
  

This is the result of the test for 18 decimals is the following :

For 15 decimals, this is the result :

You can notice the value of the variable reward is decreasing because it can’t be above type(uint16).max));

Tools Used

Forge / Manual testing

Recommended Mitigation Steps

A way to fix the issue is to change the packFixedRewards and fixedReward functions. It will be a bit less gas efficient.

    function packFixedRewards(uint256[] memory rewards) private view returns (uint256 reward) {
        if (rewards.length != (selectionSize) || rewards[0] != 0) {
            revert InvalidFixedRewardSetup();
        }

        uint256 divisor = 10 ** (IERC20Metadata(address(rewardToken)).decimals() - 1);

        for (uint8 winTier = 1; winTier < selectionSize; ++winTier) {
            reward = uint256(rewards[winTier] / divisor);
            /*             
            console.log("rewards[winTier]  %s", rewards[winTier]);
            console.log("divisor %s", divisor);
            console.log("max U16 %s", type(uint16).max);
            console.log("reward %s", reward);
            console.log("rewards[winTier] %s", rewards[winTier]); */
            //console.log("reward %s", rewards);

            if ((rewards[winTier] % divisor) != 0) {
                revert InvalidFixedRewardSetup();
            }
            /*       
            packed |= uint256(reward);
            console.log("packed %s", packed);
            console.log("=================="); */
        }
    }




    function fixedReward(uint8 winTier) public view override returns (uint256 amount) {
        if (winTier == selectionSize) {
            return _baseJackpot(initialPot);
        } else if (winTier == 0 || winTier > selectionSize) {
            return 0;
        } else {
 
            return nonJackpotFixedRewards * (10 ** (IERC20Metadata(address(rewardToken)).decimals() - 1));
        }
    }

Now the test pass :
forge test --match-test testFixedReward -vvvv

---

The text was updated successfully, but these errors were encountered:

All reactions