search

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

0 stars 0 forks source link

Principal payout #146

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#L284-L348 https://github.com/Badger-Finance/vested-aura/blob/v0.0.2/contracts/MyStrategy.sol#L370-L375 https://github.com/Badger-Finance/vested-aura/blob/v0.0.2/contracts/MyStrategy.sol#L390-L394

Vulnerability details

Impact

It's possible to treat unvested aura as bribes and an attacker may cause a withdraw of AURA from the strategy to the popint where the debt in AURA to users cannot be covered by the strategy.

Proof of Concept

Anyone can create a valuable token in which it is possible to gain execution during a transfer. Upon claimBribesFromHiddenHand invocation, one can use the execution on token transfer in hiddenHandDistributor.claim(_claims); to call manualProcessExpiredLocks or performUpkeep to claim rewards. Then, if AURA token was among the token claims in the primary call, the tokens received will be treated as a bribe instead of principal and they will be removed from the strategy, breaking the invariant that the strategy always has AURA in some form to cover debt to users.

Tools Used

Manual analysis

Recommended Mitigation Steps

Apply a nonReentrant modifier to all functions that modify AURA balance.

GalloDaSballo commented 2 years ago

Anyone can create a valuable token in which it is possible to gain execution during a transfer factually incorrect as the token would need to be whitelisted by hidden hands

one can use the execution on token transfer in hiddenHandDistributor.claim(_claims); to call manualProcessExpiredLocks or performUpkeep to claim rewards.

The warden assumes that Vault.balance() will not change if we send some tokens to the bribesProcessor this is factually incorrect.

Vault.balance calculates the amounts of tokens in:

If any of those tokens is moved, without sending more, the claim function will revert.

In lack of a POC that acknowledges this mechanism and breaks it, I must dispute

shuklaayush commented 2 years ago

Also, the claim bribes function takes as calldata the token identifiers that we're claiming so it's far-fetched that we're going to claim a malicious token