Original vulnerabilities/impacts:
The vulnerability is that there is no minimal bid amount when placing a Bid, which allows an attacker to place a bid a dust amount, and potentially front-run other bidders with minimally improved prices to get the collateral NFT at a dust value.
The mitigation is to set a minimal bid value(minBid) for each auction and only allow an auction to be created with at least the minimal value as a first bid.
This is implemented in several steps: (1) In AuctionLoanLiquidator::liquidateLoan, adding _minBid field into struct Auction (updating the corresponding Auction TypeHash, and hashing function in Hash.sol); (2) In AuctionLoanLiquidator::_placeBidChecks, adding a check on _bid value in _placeBidChecks is at least minimum bid required in struct Auction; (3) In LiquidationHandler.sol, add MIN_BID_LIQUIDATION percentage to calculate the minimum bid value based on principal amount in LiquidationHandler::_liquidateLoan()
The above mitigation resolves the issue and ensures a minimal bid amount calculated based on MIN_BID_LIQUIDATION will be set for each auction.
However, it should be noted that In LiquidationHandler.sol, an event MinBidLiquidationUpdated(uint256 newMinBid) is declared but never used. If the intention is to allow the owner to change MIN_BID_LIQUIDATION , then consider changing MIN_BID_LIQUIDATION to be mutable and add onlyOwner function to change this value.
Lines of code
Vulnerability details
C4 Issue
M-11: AuctionLoanLiquidator#placeBid can be DoS
Comments
Original vulnerabilities/impacts: The vulnerability is that there is no minimal bid amount when placing a Bid, which allows an attacker to place a bid a dust amount, and potentially front-run other bidders with minimally improved prices to get the collateral NFT at a dust value.
Mitigation
Fix: https://github.com/pixeldaogg/florida-contracts/pull/377/files
The mitigation is to set a minimal bid value(minBid) for each auction and only allow an auction to be created with at least the minimal value as a first bid.
This is implemented in several steps: (1) In AuctionLoanLiquidator::liquidateLoan, adding _minBid field into struct Auction (updating the corresponding Auction TypeHash, and hashing function in Hash.sol); (2) In AuctionLoanLiquidator::_placeBidChecks, adding a check on _bid value in _placeBidChecks is at least minimum bid required in struct Auction; (3) In LiquidationHandler.sol, add MIN_BID_LIQUIDATION percentage to calculate the minimum bid value based on principal amount in LiquidationHandler::_liquidateLoan()
The above mitigation resolves the issue and ensures a minimal bid amount calculated based on MIN_BID_LIQUIDATION will be set for each auction.
However, it should be noted that In LiquidationHandler.sol, an
event MinBidLiquidationUpdated(uint256 newMinBid)
is declared but never used. If the intention is to allow the owner to change MIN_BID_LIQUIDATION , then consider changing MIN_BID_LIQUIDATION to be mutable and add onlyOwner function to change this value.Test
The revised test is passing.
Conclusion
LGTM