Arithmetic Error - `manualRebalance` function has multiple arithmetic bugs

[code423n4]

Handle

tabish

Vulnerability details

• In short there are 2 errors in manualRebalance function : a ratio (currentLockRatio) has been compared to balance (newLockRatio) https://github.com/code-423n4/2021-09-bvecvx/blob/1d64bd58c7a4224cc330cef283561e90ae6a3cf5/veCVX/contracts/veCVXStrategy.sol#L477 and at another point in the same function subtracted https://github.com/code-423n4/2021-09-bvecvx/blob/1d64bd58c7a4224cc330cef283561e90ae6a3cf5/veCVX/contracts/veCVXStrategy.sol#L488 .

1> Expanding on the first one - checking newLockRatio < = currentLockRatio is incorrect as newLockRatio is the new total amount of CVX to lock in the locker and we are comparing it with a the ratio of (cvx in (locker * 10 **18)/totalCVXBalance). https://github.com/code-423n4/2021-09-bvecvx/blob/1d64bd58c7a4224cc330cef283561e90ae6a3cf5/veCVX/contracts/veCVXStrategy.sol#L477

Solution: Instead the if should be like if (newLockRatio <= balanceInLock) {

2> Expanding on the second one - Calculation mistake in calculating cvxToLock in manualRebalance function. https://github.com/code-423n4/2021-09-bvecvx/blob/1d64bd58c7a4224cc330cef283561e90ae6a3cf5/veCVX/contracts/veCVXStrategy.sol#L488

so lets say the total balanceInLock = 100 * 1018 wei totalCVXBalance = 200 * 10*18 wei toLock = 7_000 then currentLockRatio = 5 1017 and newLockRatio = 140 * 10**18 wei

to calculate the cvxToLock in the function on line #488 we do this uint256 cvxToLock = newLockRatio.sub(currentLockRatio); which gives an incorrect answer as newLockRatio is the total funds to lock into the CVX Locker according to the new ratio.

Solution: Therefore cvxToLock should be calculated as uint256 cvxToLock = newLockRatio.sub(balanceInLock);

Also the variable newLockRatio should be named newLockAmount and currentLockRatio definition and references should be removed from the manualRebalance function

[GalloDaSballo]

Agree with the finding, the math is wrong

We will mitigate by deleting the function and usingmanualSendbCVXToVault as way to manually rebalance

[GalloDaSballo]

We mitigated by rewriting manualRebalance following the advice of the warden

[ghoul-sol]

duplicat of #47