c4-bot-3 commented 3 months ago

Lines of code

Vulnerability details

C4 Issue

M-08: Borrower signature could be reused in emitLoan()

Comments

Original vulnerabilities: The key vulnerability is struct ExecutionData() doesn’t implement a nonce to prevent signature(LoanExecutionData.borrowerOfferSignature) replay.

Original impacts: The attack scenario described in the issue is emitLoan() can be invoked a second time by an attacker with a previously signed LoanExecutionData to take out a loan for a borrower(victim).

It should be noted that for the attack to succeed some conditions need to be met: (1)ExecutionData set by the original borrower cannot expire; (2) The original borrower would also need to approve MultiSourceLoan for the required collateral NFT.

Mitigation

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

//src/interfaces/loans/IMultiSourceLoan.sol

    /// @dev It's advised that borrowers only set an expirationTime close to the actual time they will execute the loan
    ///      to avoid replays. 
...

The sponsor added code comments to suggest the borrower needs to provide a realistic expiration time for the original ExecutionData such that by the time the original loan’s duration passed, the Execution data would have already expired. No changes to the code implement is provided in this mitigation.

Due to the strict conditions that the attack requires, it’s user/borrower's responsibility to enter a realistic expiration time. Also MultisourceLoan.sol needs to still have the approval from the borrower and the borrower needs to still own the NFT collateral to allow the attacker’s emitLoan() call to succeed.

For these reasons, I consider this issue to be mitigated by the sponsor’s documentation of the risks in code comments.

Conclusion

LGTM

c4-judge commented 3 months ago

alex-ppg marked the issue as satisfactory