code-423n4 / 2024-04-gondi-findings

0 stars 0 forks source link

validateOffer() shouldn't be able to use getCollectedFees #32

Closed c4-bot-10 closed 7 months ago

c4-bot-10 commented 7 months ago

Lines of code

https://github.com/code-423n4/2024-04-gondi/blob/b9863d73c08fcdd2337dc80a8b5e0917e18b036c/src/lib/pools/Pool.sol#L396

Vulnerability details

Vulnerability details

in validateOffer() can use getCollectedFees for payments

    function validateOffer(bytes calldata _offer, uint256 _protocolFee) external override onlyAcceptedCallers {
        if (!isActive) {
            revert PoolStatusError();
        }
@>      uint256 currentBalance = asset.balanceOf(address(this)) - getAvailableToWithdraw;
        uint256 baseRateBalance = IBaseInterestAllocator(getBaseInterestAllocator).getAssetsAllocated();
        uint256 undeployedAssets = currentBalance + baseRateBalance;
        (uint256 principalAmount, uint256 apr) = IPoolOfferHandler(getUnderwriter).validateOffer(
            IBaseInterestAllocator(getBaseInterestAllocator).getBaseAprWithUpdate(), _offer
        );

currentBalance contains getCollectedFees.

But _getUndeployedAssets() subtracts getCollectedFees.

This may cause _getUndeployedAssets() to underflow

    function _getUndeployedAssets() private view returns (uint256) {
        return asset.balanceOf(address(this)) + IBaseInterestAllocator(getBaseInterestAllocator).getAssetsAllocated()
@>          - getAvailableToWithdraw - getCollectedFees;
    }

Example: balanceOf(address(this)) = 150 getCollectedFees = 50 getAvailableToWithdraw = 0 IBaseInterestAllocator(getBaseInterestAllocator).getAssetsAllocated() = 0 offer.principalAmount = 150

This way when the offer is executed, it is transferred away 150 _getUndeployedAssets() = 0 + 0 - 0 - 50 = -50 (underflow)

Impact

_getUndeployedAssets() underflow will cause most of ERC4626 methods to fail.

Recommended Mitigation

    function validateOffer(bytes calldata _offer, uint256 _protocolFee) external override onlyAcceptedCallers {
        if (!isActive) {
            revert PoolStatusError();
        }
-       uint256 currentBalance = asset.balanceOf(address(this)) - getAvailableToWithdraw;
+      uint256 currentBalance =  asset.balanceOf(address(this)) - getAvailableToWithdraw - getCollectedFees;
        uint256 baseRateBalance = IBaseInterestAllocator(getBaseInterestAllocator).getAssetsAllocated();
        uint256 undeployedAssets = currentBalance + baseRateBalance;
        (uint256 principalAmount, uint256 apr) = IPoolOfferHandler(getUnderwriter).validateOffer(
            IBaseInterestAllocator(getBaseInterestAllocator).getBaseAprWithUpdate(), _offer
        );

Assessed type

Context

c4-judge commented 7 months ago

0xA5DF marked the issue as primary issue

c4-judge commented 7 months ago

0xA5DF marked the issue as duplicate of #44

c4-judge commented 7 months ago

0xA5DF marked the issue as satisfactory