search

code-423n4 / 2022-04-backd-findings

6 stars 4 forks source link

Error in _rebalance logic will result in limited funds in strategy to earn yield #205

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-04-backd/blob/c856714a50437cb33240a5964b63687c9876275b/backd/contracts/vault/Vault.sol#L784

Vulnerability details

The vault is responsible for delegating funds to the strategy. But it has a multifaceted role.As,after a deposit is made from the pool, it checks whether a rebalancing should be done on the strategy,ie, if there's an excess or deficit, then the difference is moved to the vault and vice-a-versa.

Consequently,the current calculation within _rebalance () doesn't capture this rebalancing process effectively.Thus, an erroneously high value would be withdrawn :

https://github.com/code-423n4/2022-04-backd/blob/c856714a50437cb33240a5964b63687c9876275b/backd/contracts/vault/Vault.sol#L784

The full amount of the underlying would be withdrawn when there would be no funds needed to rebalance. for eg, withdrawnAmt =5000-0.

On the other hand, if the target is non-zero,for instance, 5000-100=4900,it would withdraw more than is necessary to reduce the excess in the strategy and rebalance the vault.

Additionally, the output in the call to _computeAllocation() would inappropriately reduce the allocation.

kartoonjoy commented 2 years ago

Recommended fix submitted after contest close. Note for judge: Comment is being added as a value-add to the sponsor and should not affect weighting since it was provided after the contest deadline.

change the aforementioned to :

uint256 withdrawnAmount=allocatedUnderlying-(allocatedUnderlying-target) ;

chase-manning commented 2 years ago

Target is the target amount to be held in the strategy. Not the target amount to be withdrawn. Submitter has misunderstood the function of this value.