`rebalance()` calculates `sharesToMint` by rounding-down against the protocol's favour

[c4-bot-3]

Lines of code

https://github.com/code-423n4/2024-05-bakerfi/blob/main/contracts/core/Vault.sol#L153-L158

Vulnerability details

Description

The Vault::rebalance() function rounds-down the sharesToMint against the protocol's favour. It ought to be rounded-up to avoid loss of funds for the protocol.

                    uint256 feeInEthScaled = uint256(balanceChange) *
                        settings().getPerformanceFee();
                    uint256 sharesToMint = (feeInEthScaled * totalSupply()) /
                        _totalAssets(maxPriceAge) /
                        PERCENTAGE_PRECISION;
                    _mint(settings().getFeeReceiver(), sharesToMint);

Impact

Loss of funds for the protocol.

Tools Used

Manual review

Recommended Mitigation Steps

Round up in favour of the protocol. A library like solmate can be used which has mulDivUp:

-                   uint256 sharesToMint = (feeInEthScaled * totalSupply()) /
-                       _totalAssets(maxPriceAge) /
-                       PERCENTAGE_PRECISION;
+                   uint256 sharesToMint = feeInEthScaled.mulDivUp(totalSupply(), _totalAssets(maxPriceAge) * PERCENTAGE_PRECISION);

Assessed type

Math

[c4-judge]

0xleastwood marked the issue as primary issue

[0xleastwood]

This is not medium severity, there is minimal rounding here that does not cause significant leakage of funds. Downgrading to QA because rounding should always be done in favour of the protocol.

[c4-judge]

0xleastwood changed the severity to QA (Quality Assurance)

[t0x1cC0de]

Hi,
Please refer my comment under Issue 4 here.

Thanks

[ickas]

Fixed → https://github.com/baker-fi/bakerfi-contracts/pull/52