code-423n4 / 2022-05-vetoken-findings

1 stars 1 forks source link

QA Report #232

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Low

  1. Title : Value of certain range can be used instead of nothing

at Booster.sol (Line.233) was used :

        //values must be within certain ranges

but at certain range was not used, so it can be deleted as it should be or you can use implementation down below :

    function setFees(uint256 _lockFees, uint256 _stakerFees, uint256 _callerFees, uint256 _platform) 
    external {
        require(msg.sender==feeManager, "!auth");

        uint256 total = _lockFees.add(_stakerFees).add(_callerFees).add(_platform);
        require(total <= MaxFees, ">MaxFees");

        //values must be within certain ranges     
        if(_lockFees >= 1000 && _lockFees <= 1500 
            && _stakerFees >= 300 && _stakerFees <= 600
            && _callerFees >= 10 && _callerFees <= 100
            && _platform <= 200) {
            lockIncentive = _lockFees;
            stakerIncentive = _stakerFees;
            earmarkIncentive = _callerFees;
            platformFee = _platform;
        }

so the value was used to be on certain range and not gonna do unusual behavior.

  1. Title : Comment was not the same as Actual code
        //token factory can also be immutable

on this code was using token factory can also be immutable but at the actual code can't be an immutable, so comment can be removed or it can be changed instead.

Tool Used

Manual Review

  1. Title : Unnecessary Code

https://github.com/code-423n4/2022-05-vetoken/blob/2d7cd1f6780a9bcc8387dea8fecfbd758462c152/contracts/VoterProxy.sol#L282

        require(success, "!success");

this checked was unnecessary since it would pass to return, so instead for being checked and no checked was used. it can be deleted instead and pass it to return.

Non Critical

  1. Title : Typo Comment

This cam be changed as it should be for better readibility

1.) File : contracts/VeAssetDepositor.sol (Line.93)

changed into amount

        //increase ammount

2.) File : contracts/Booster.sol (Line.31)

changed into platfrom

    // platoform fee
  1. Title : simplify the number of maxSupply

https://github.com/code-423n4/2022-05-vetoken/blob/2d7cd1f6780a9bcc8387dea8fecfbd758462c152/contracts/VeTokenMinter.sol#L15

    uint256 public constant maxSupply = 30 * 1000000 * 1e18; //30mil

changed to :

uint256 public constant maxSupply = 3e25 //30mil
  1. Title : change add,mul,div into math operator

Since update into pragma ^0.8.0

Optional: If you use SafeMath or a similar library, change x.add(y) to x + y, x.mul(y) to x * y etc.

1.) File : contracts/VeAssetDepositor.sol (Line.140)

                _amount = _amount.add(incentiveVeAsset);

2.) File : contracts/VeAssetDepositor.sol (Line.147)

              uint256 callIncentive = _amount.mul(lockIncentive).div(FEE_DENOMINATOR);

3.) File : contracts/VeAssetDepositor.sol (Line.148)

              _amount = _amount.sub(callIncentive);

4.) File : contracts/VeAssetDepositor.sol (Line.151)

              incentiveVeAsset = incentiveVeAsset.add(callIncentive);

5.) File : contracts/Booster.sol (Line.228)

             uint256 total = _lockFees.add(_stakerFees).add(_callerFees).add(_platform).add(

6.) File : contracts/Booster.sol (Lines.518-523)

            uint256 _lockIncentive = veAssetBal.mul(lockIncentive).div(FEE_DENOMINATOR);
            uint256 _stakerIncentive = veAssetBal.mul(stakerIncentive).div(FEE_DENOMINATOR);
            uint256 _stakerLockIncentive = veAssetBal.mul(stakerLockIncentive).div(
                FEE_DENOMINATOR
            );
            uint256 _callIncentive = veAssetBal.mul(earmarkIncentive).div(FEE_DENOMINATOR);

7.) File : contracts/Booster.sol (Lines.528-529)

            uint256 _platform = veAssetBal.mul(platformFee).div(FEE_DENOMINATOR);
            veAssetBal = veAssetBal.sub(_platform);

8.) File : contracts/Booster.sol (Lines.534-L538)

            veAssetBal = veAssetBal
                .sub(_lockIncentive)
                .sub(_callIncentive)
                .sub(_stakerIncentive)
                .sub(_stakerLockIncentive);

9.) File : contracts/Booster.sol (Lines.582-584)

           uint256 _lockFeesIncentive = _balance.mul(lockFeesIncentive).div(FEE_DENOMINATOR);
           uint256 _stakerLockFeesIncentive = _balance.mul(stakerLockFeesIncentive).div(
                FEE_DENOMINATOR

10.) File : contracts/BaseRewardPool.sol (Line.149)

           return Math.min(block.timestamp, periodFinish);

11.) File : contracts/BaseRewardPool.sol (Lines.157-159)

            rewardPerTokenStored.add(
                lastTimeRewardApplicable().sub(lastUpdateTime).mul(rewardRate).mul(1e18).div(
                    totalSupply()

12.) File : contracts/BaseRewardPool.sol (Lines.166-169)

            balanceOf(account)
                .mul(rewardPerToken().sub(userRewardPerTokenPaid[account]))
                .div(1e18)
                .add(rewards[account]);

13.) File : contracts/BaseRewardPool.sol (Lines.180-181)

           _totalSupply = _totalSupply.add(_amount);
           _balances[msg.sender] = _balances[msg.sender].add(_amount);

14.) File : contracts/BaseRewardPool.sol (Lines.204-205)

           _totalSupply = _totalSupply.add(_amount);
           _balances[_for] = _balances[_for].add(_amount);

15.) File : contracts/BaseRewardPool.sol (Lines.L222-223)

           _totalSupply = _totalSupply.sub(amount);
           _balances[msg.sender] = _balances[msg.sender].sub(amount);

16.) File : contracts/BaseRewardPool.sol (Lines.249-250))

            _totalSupply = _totalSupply.sub(amount);
            _balances[msg.sender] = _balances[msg.sender].sub(amount);

17.) File : contracts/BaseRewardPool.sol (Line.296))

            queuedRewards = queuedRewards.add(_amount);

18.) File : contracts/BaseRewardPool.sol (Line.303))

             _rewards = _rewards.add(queuedRewards);

19.) File : contracts/BaseRewardPool.sol (Line.312))

             uint256 elapsedTime = block.timestamp.sub(periodFinish.sub(duration));

20.) File : contracts/BaseRewardPool.sol (Line.315))

            uint256 queuedRatio = currentAtNow.mul(1000).div(_rewards);

21.) File : contracts/BaseRewardPool.sol (Line.328))

            historicalRewards = historicalRewards.add(reward);

22.) File : contracts/BaseRewardPool.sol (Line.330))

             rewardRate = reward.div(duration);

23.) File : contracts/BaseRewardPool.sol (Lines.332-335))

            uint256 remaining = periodFinish.sub(block.timestamp);
            uint256 leftover = remaining.mul(rewardRate);
            reward = reward.add(leftover);
            rewardRate = reward.div(duration);

24.) File : contracts/VE3DRewardPool.sol (Lines.176-182)

            rewardTokenInfo[_rewardToken].rewardPerTokenStored.add(
                lastTimeRewardApplicable(_rewardToken)
                    .sub(rewardTokenInfo[_rewardToken].lastUpdateTime)
                    .mul(rewardTokenInfo[_rewardToken].rewardRate)
                    .mul(1e18)
                    .div(supply)
            );

25.) File : contracts/VE3DRewardPool.sol (Lines.187-194)

               balanceOf(account)
                .mul(
                    rewardPerToken(_rewardToken).sub(
                        rewardTokenInfo[_rewardToken].userRewardPerTokenPaid[account]
                    )
                )
                .div(1e18)
                .add(rewardTokenInfo[_rewardToken].rewards[account]);

26.) File : contracts/VE3DRewardPool.sol (Line.202)

              uint256 fees = r.mul(depositFeeRate).div(FEE_DENOMINATOR);

27.) File : contracts/VE3DRewardPool.sol (Lines.219-221)

         _totalSupply = _totalSupply.add(_amount);
          //add to sender balance sheet
        _balances[msg.sender] = _balances[msg.sender].add(_amount);

28.) File : contracts/VE3DRewardPool.sol (Lines.243-245)

         _totalSupply = _totalSupply.add(_amount);
          //add to _for's balance sheet
        _balances[msg.sender] = _balances[msg.sender].add(_amount);

29.) File : contracts/VE3DRewardPool.sol (Lines.353-354)

        uint256 elapsedTime = block.timestamp.sub(
        rewardTokenInfo[_rewardToken].periodFinish.sub(duration)

30.) File : contracts/VE3DRewardPool.sol (Lines.373-382)

kartoonjoy commented 2 years ago

Per warden Funen, 5. Title : simplify the number of maxSupply needed to have the link to the code added. This has now been done. Thanks

GalloDaSballo commented 2 years ago

Title : Value of certain range can be used instead of nothing

Valid Low (May raise to Med)

Title : Comment was not the same as Actual code

Valid NC

Title : Unnecessary Code

Disagree as the sponsor wants to revert on failure

Title : Typo Comment

Valid NC

Title : simplify the number of maxSupply

Valid Refactoring

Title : change add,mul,div into math operator

Valid Refactoring

Good short and sweet report 1L, 2R, 2Nc

GalloDaSballo commented 2 years ago

Title : Value of certain range can be used instead of nothing -> Dup of M-11

Updated score: 2R, 2NC