Badger-Vested-Aura contest - tabish's results

Lines of code

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

Vulnerability details

Impact

Detailed description of the impact of this finding.

getReward(address account) function of Aura Locker is an external function therefore can be called by anyone by passing in the address of strategy and transferring the rewards to the strategy. harvest function takes into account the initial balance of reward token before claiming reward and calculating auraBalEarned, therefore not taking into account any rewards which were transferred to the strategy directly.

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

uint256 auraBalBalanceBefore = AURABAL.balanceOf(address(this));

// Claim auraBAL from locker
LOCKER.getReward(address(this));

harvested = new TokenAmount[](1);
harvested[0].token = address(AURA);

uint256 auraBalEarned = AURABAL.balanceOf(address(this)).sub(auraBalBalanceBefore);

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.

Attack vector -

• random user, calls getReward(address of strategy) and reward token (auraBal) get transferred to strategy
• harvest function does not take into account the transferred reward token (auraBal) and just processes the newly claimed rewards.

Also there is no method to move the reward token (auraBAL) without an upgrade as it is a protected token https://github.com/Badger-Finance/vested-aura/blob/d504684e4f9b56660a9e6c6dfb839dcebac3c174/contracts/MyStrategy.sol#L164

Recommended Mitigation Steps

harvest should take into account all the gained auraBAL before autocompounding it.