c4-bot-3 commented 3 months ago

Lines of code

Vulnerability details

C4 Issue

H-14: mergeTranches()/refinancePartial() lack of nonReentrant

Comments

Original vulnerabilities: mergeTranches() and refinancePartial() don’t have nonReentrant modifiers.

This is risky because mergeTranches() and refinancePartial() are publicly accessible and allow for a custom contract call IOfferValidator(CustomContract).validateOffer().

Original impacts: This allows for reentrancy and it may create an impact where the same NFT is used in multiple loans at the same time.

Mitigation

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

//src/lib/loans/MultiSourceLoan.sol
    function refinancePartial(RenegotiationOffer calldata _renegotiationOffer, Loan memory _loan)
        external
|>      nonReentrant
        returns (uint256, Loan memory)
    {
...
    function mergeTranches(uint256 _loanId, Loan memory _loan, uint256 _minTranche, uint256 _maxTranche)
        external
|>      nonReentrant
        returns (uint256, Loan memory)
    {

nonReentrant modifier is added to mergeTranches() and refinancePartial(). This eliminates the attack vector and resolve the issue.

Conclusion

LGTM

c4-judge commented 3 months ago

alex-ppg marked the issue as satisfactory