sherlock-audit / 2022-09-notional-judging

4 stars 2 forks source link

ctf_sec - BAL_TOKEN and AURA_TOKEN are hardcoded into the LIQUIDITY_GAUGE token reward in AuraStakingMixin.sol #95

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

ctf_sec

high

BAL_TOKEN and AURA_TOKEN are hardcoded into the LIQUIDITY_GAUGE token reward in AuraStakingMixin.sol

Summary

BAL_TOKEN and AURA_TOKEN are hardcoded into the LIQUIDITY_GAUGE token reward in AuraStakingMixin.sol

Vulnerability Detail

BAL_TOKEN and AURA_TOKEN are hardcoded into the LIQUIDITY_GAUGE token reward in AuraStakingMixin.sol

    function _rewardTokens() private view returns (IERC20[] memory tokens) {
        uint256 rewardTokenCount = LIQUIDITY_GAUGE.reward_count() + 2;
        tokens = new IERC20[](rewardTokenCount);
        tokens[0] = BAL_TOKEN;
        tokens[1] = AURA_TOKEN;
        for (uint256 i = 2; i < rewardTokenCount; i++) {
            tokens[i] = IERC20(LIQUIDITY_GAUGE.reward_tokens(i - 2));
        }
    }

the function assume that the token reward supported by LIQUIDITY_GAUGE must includes BAL_TOKEN and AURA_TOKEN, however, this is subject to change.

https://etherscan.io/address/0x3b8cA519122CdD8efb272b0D3085453404B25bD0?utm_source=immunefi#code

this Smart Contract - LiquidityGaugeV5 listed from Balancer immunefi bug bounty is an example.

We can see that if we read the smart contract,

https://etherscan.io/address/0x3b8cA519122CdD8efb272b0D3085453404B25bD0#readContract

the reward_count return 0.

Impact

If a list of reward token is not properly set up,

then the function

function claimRewardTokens() 

fails when we transfer the fee

   uint256 feeAmount = claimedBalances[i] * feePercentage / BalancerConstants.VAULT_PERCENT_BASIS;
   rewardTokens[i].checkTransfer(FEE_RECEIVER, feeAmount);
   claimedBalances[i] -= feeAmount;

Code Snippet

https://github.com/sherlock-audit/2022-09-notional/blob/main/leveraged-vaults/contracts/vaults/balancer/mixins/AuraStakingMixin.sol#L41-L48

Tool used

Manual Review

Recommendation

We recommend the project not hard code the token type, instead we read the supported directly from the contract

    function _rewardTokens() private view returns (IERC20[] memory tokens) {
        uint256 rewardTokenCount = LIQUIDITY_GAUGE.reward_count();
        tokens = new IERC20[](rewardTokenCount);
        for (uint256 i = 0; i < rewardTokenCount; i++) {
            tokens[i] = IERC20(LIQUIDITY_GAUGE.reward_tokens(i));
        }
    }
jeffywu commented 1 year ago

@weitianjie2000

jeffywu commented 1 year ago

BAL_TOKEN and AURA_TOKEN are added to the list of tokens to check in addition to the reward tokens returned by the LIQUIIDITY_GAUGE. If BAL or AURA are not transferred as a result of the reward claim, it would simply emit a 0 balance change for that token. There would be no other side effects of this hardcoding.