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

0 stars 0 forks source link

BADGER bribes can not be claimed #94

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/Badger-Finance/vested-aura/blob/v0.0.2/contracts/MyStrategy.sol#L428

Vulnerability details

_sendBadgerToTree will send BADGER twice and therefore fail. It is sending it to the BADGER_TREE in _sendBadgerToTree, and then continues to send the same amount to the vault in _processExtraToken.

Impact

BADGER rewards cannot be claimed. The contract is trying to send same amount twice - therefore the second transfer will fail. (Unless there is extra BADGER in the contract, in which case there will be accounting errors.)

Proof of Concept

This is _sendBadgerToTree:

    function _sendBadgerToTree(uint256 amount) internal {
        IERC20Upgradeable(BADGER).safeTransfer(BADGER_TREE, amount);
        _processExtraToken(address(BADGER), amount);
    }

And this is BaseStrategy's _processExtraToken:

    function _processExtraToken(address _token, uint256 _amount) internal {
        require(_token != want, "Not want, use _reportToVault");
        require(_token != address(0), "Address 0");
        require(_amount != 0, "Amount 0");

        IERC20Upgradeable(_token).safeTransfer(vault, _amount);
        IVault(vault).reportAdditionalToken(_token);
    }

As we can see in the safeTransfer lines of the two functions, the BADGER amount is being sent in _sendBadgerToTree to BADGER_TREE and then it is being sent in _processExtraToken to the vault.

Recommended Mitigation Steps

I am not sure, but to my understanding you don't need to call _processExtraToken in _sendBadgerToTree, but only call IVault(vault).reportAdditionalToken(_token).

GalloDaSballo commented 2 years ago

Dup of #2