Collected fees are never transferred out of Pool contract

[c4-bot-7]

Lines of code

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

Vulnerability details

Impact

Lenders in the Gondi protocol could be EOA or Gondi Pool. The Gondi Pool, an ERC4626, allows anyone to deposit funds and earn yield from lending on Gondi. When a loan is repaid or liquidated, the pool deducts a fee from the received amount before adding the rest to the pool balance. As shown in the loanRepayment() function, the fees are calculated by calling processFees() and then added to getCollectedFees. After that, the accounting function _loanTermination() is called with the amount being received - fees.

However, this fee is credited to getCollectedFees but never transferred out of the pool. As a result, these funds remain locked in the contract indefinitely.

function loanRepayment(
    uint256 _loanId,
    uint256 _principalAmount,
    uint256 _apr,
    uint256,
    uint256 _protocolFee,
    uint256 _startTime
) external override onlyAcceptedCallers {
    uint256 netApr = _netApr(_apr, _protocolFee);
    uint256 interestEarned = _principalAmount.getInterest(netApr, block.timestamp - _startTime);
    uint256 received = _principalAmount + interestEarned;
    uint256 fees = IFeeManager(getFeeManager).processFees(_principalAmount, interestEarned);
    getCollectedFees += fees; // @audit getCollectedFees is never transfer out
    _loanTermination(msg.sender, _loanId, _principalAmount, netApr, interestEarned, received - fees);
}

Proof of Concept

The processFees() function only calculates the fee but doesn't transfer anything.

function processFees(uint256 _principal, uint256 _interest) external view returns (uint256) {
    /// @dev cached
    Fees memory __fees = _fees;
    return _principal.mulDivDown(__fees.managementFee, PRECISION)
        + _interest.mulDivDown(__fees.performanceFee, PRECISION);
}

Then after getCollectedFees is credited for fees, we can see this getCollectedFees is never transferred out of the pool.

Tools Used

Manual Review

Recommended Mitigation Steps

Add a function to collect the credited fees getCollectedFees from the pool in the FeeManager contract.

Assessed type

Other

[c4-judge]

0xA5DF marked the issue as primary issue

[0xA5DF]

TODO: severity

[0xend]

Not sure if it's high - tend to think as high as those that would compromise user's assets. Definitely an issue though.

[c4-judge]

0xA5DF changed the severity to 2 (Med Risk)

[0xA5DF]

Marking as med as fees falls under the definition 'leak of value'

[c4-judge]

0xA5DF marked the issue as selected for report

[c4-judge]

0xA5DF marked the issue as satisfactory

[0xend]

https://github.com/pixeldaogg/florida-contracts/pull/366