c4-bot-8 commented 3 months ago

Lines of code

Vulnerability details

C4 Issue

M-03: Function addNewTranche() should use protocolFee from Loan struct

Comments

Original vulnerabilities/impacts: addNewTranche() uses the live protocolFee fraction to charge protocol fee from the renegotiationOffer lender. This might be the incorrect protocolFee fraction if the protocol fee has been updated by admin.

For comparison, other renegotiation flow such as refinanceFull, and refinancePartial will use the protocolFee fraction stored from the original loan struct.

Mitigation

Fix: https://github.com/pixeldaogg/florida-contracts/pull/363/files

//src/lib/loans/MultiSourceLoan.sol
    function addNewTranche(
        RenegotiationOffer calldata _renegotiationOffer,
        Loan memory _loan,
        bytes calldata _renegotiationOfferSignature
    ) external nonReentrant returns (uint256, Loan memory) {
...
        if (_renegotiationOffer.fee != 0) {
            /// @dev Cached
            ERC20(_loan.principalAddress).safeTransferFrom(
                _renegotiationOffer.lender,
                _protocolFee.recipient,
|>              _renegotiationOffer.fee.mulDivUp(_loan.protocolFee, _PRECISION)
            );
        }

The mitigation is to use the protocolFee fraction from the stored loan struct, instead of the live protocolFee value. This ensures protocolFee is charged consistently among other similar renegotiation flows using the original loan’s data.

This eliminates the original vulnerability and resolves the issue.

Conclusion

LGTM

c4-judge commented 3 months ago

alex-ppg marked the issue as satisfactory