search

code-423n4 / 2021-08-yield-findings

1 stars 0 forks source link

Incorrect type of uint parameter is used in event #19

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

JMukesh

Vulnerability details

Impact

incorrect type of uint parameter is used during emitting an event in ERC20Reward.sol

  1. event RewardsSet(IERC20 rewardsToken, uint32 start, uint32 end, uint256 rate);

    here rate is of uint256 type but in function RewardSet() rate is passed as uint96 in event parameter

  2. event RewardsPerTokenUpdated(uint256 accumulated);

    rewardsPerToken_.accumulated is of uint128 which is passed in event

  3. event UserRewardsUpdated(address user, uint256 userRewards, uint256 paidRewardPerToken);

userRewards.accumulated is of uint128, userRewards.checkpoint is of uint128

Proof of Concept

https://github.com/code-423n4/2021-08-yield/blob/4dc46470e616dd0cbd9db9b4742e36c4d809e02c/contracts/utils/token/ERC20Rewards.sol#L97

https://github.com/code-423n4/2021-08-yield/blob/4dc46470e616dd0cbd9db9b4742e36c4d809e02c/contracts/utils/token/ERC20Rewards.sol#L119

https://github.com/code-423n4/2021-08-yield/blob/4dc46470e616dd0cbd9db9b4742e36c4d809e02c/contracts/utils/token/ERC20Rewards.sol#L134

Tools Used

manual review

Recommended Mitigation Steps

same type of uint should be used during declaration and emitting event

alcueca commented 3 years ago

That's right, but it's zero risk because the variables fed to the events cast up. uint256(uint96) has no risk of overflow.

Actually, there might be a gas optimization on casting all event parameters up to one word :thinking:

ghoul-sol commented 3 years ago

One should keep proper types but this does no harm so I'll make it non-critical