Lines of code

https://github.com/Badger-Finance/vested-aura/blob/d504684e4f9b56660a9e6c6dfb839dcebac3c174/contracts/MyStrategy.sol#L107-L113 https://github.com/Badger-Finance/vested-aura/blob/d504684e4f9b56660a9e6c6dfb839dcebac3c174/contracts/MyStrategy.sol#L161-L166 https://github.com/Badger-Finance/vested-aura/blob/d504684e4f9b56660a9e6c6dfb839dcebac3c174/contracts/MyStrategy.sol#L56-L74

Vulnerability details

Impact

because before contract initialization AURA token is not in getProtectedTokens() list then it's possible to call sweepRewardToken() with AURA as token and transfer AURA balance of contract to bribesProcessor address which if it was uninitialized and was 0x0 fund would be lost. So fund could be lost if sweepRewardToken() is get called before initialization and there is no check that contract is initialized and AURA is not in protected token list before initialization.

Proof of Concept

This is getProtectedTokens() code:

    function getProtectedTokens() public view virtual override returns (address[] memory) {
        address[] memory protectedTokens = new address[](2);
        protectedTokens[0] = want; // AURA
        protectedTokens[1] = address(AURABAL);
        return protectedTokens;
    }

As you can see the list has want and AURABAL token. this is initialize() code:

    function initialize(address _vault) public initializer {
        assert(IVault(_vault).token() == address(AURA));

        __BaseStrategy_init(_vault);

        want = address(AURA);

        /// @dev do one off approvals here
        // Permissions for Locker
        AURA.safeApprove(address(LOCKER), type(uint256).max);

        AURABAL.safeApprove(address(BALANCER_VAULT), type(uint256).max);
        WETH.safeApprove(address(BALANCER_VAULT), type(uint256).max);

        // Set Safe Defaults
        withdrawalSafetyCheck = true;

        // Process locks on reinvest is best left false as gov can figure out if they need to save that gas
    }

As you can see want value set to AURA in initialization and before that it has 0x0 value so AURA is not in protected tokens list before initialization. This is sweepRewardToken() code:

    function sweepRewardToken(address token) public nonReentrant {
        _onlyGovernanceOrStrategist();
        _onlyNotProtectedTokens(token);

        uint256 toSend = IERC20Upgradeable(token).balanceOf(address(this));
        _handleRewardTransfer(token, toSend);
    }

As you can see it's callable by everyone and it checks that token is not in protected tokens list. so if someone calls it before initialization with AURA as token, then code would call _handleRewardTransfer() and transfers all the AURA balance of contract to bribesProcessor which could be 0x0 and funds would be lost.

Tools Used

VIM

Recommended Mitigation Steps

add AURA to protected list all the times or check for initialization state in logics.

code-423n4 / 2022-06-badger-findings