ACCIDENTAL ZERO BIDBPS ENTRY LEADING TO NON-COMPETITIVE BIDS IN AUCTIONTYPE.LARGE_RISK_FUND

[code423n4]

Lines of code

https://github.com/code-423n4/2023-05-venus/blob/main/contracts/Shortfall/Shortfall.sol#L158-L202 https://github.com/code-423n4/2023-05-venus/blob/main/contracts/Shortfall/Shortfall.sol#L209-L259

Vulnerability details

Impact

If a user accidentally enters a bidBps value of zero when the auction.auctionType is set to AuctionType.LARGE_RISK_FUND, their bid would be considered as the winner bid. This is because, for AuctionType.LARGE_RISK_FUND, the winning bid is the one with the lowest bidBps. In a scenario where there are no other bids or all other bids are higher, this accidental zero bid could potentially win the auction. However, this would most likely result in a significant financial loss for the auction initiator since the auctioned assets would be sold for a minimal (zero in this case) amount.

Proof of Concept

Here's an example scenario:

1. Alice initiates an auction with auction.auctionType set to AuctionType.LARGE_RISK_FUND.
2. Bob intends to place a bid but accidentally enters bidBps as 0.
3. Bob's bid instantly becomes the winning bid.
4. As a result, zero riskFundBidAmount of the auctioned assets is sold for the entire debt, leading to a significant financial loss tom Bob.

            riskFundBidAmount = (auction.seizedRiskFund * auction.highestBidBps) / MAX_BPS;
        }

        uint256 transferredAmount = riskFund.transferReserveForAuction(comptroller, riskFundBidAmount);
        IERC20Upgradeable(convertibleBaseAsset).safeTransfer(auction.highestBidder, riskFundBidAmount);

Recommended Mitigation Steps

1. Input validation: Implement stricter validation for the bidBps input to prevent users from accidentally entering a zero bid. For example, the smart contract could require that the bidBps must be within a specific range that excludes zero.

2. User confirmation: Before finalizing the transaction, require user confirmation for the entered bidBps, especially when it's unusually low. This can be implemented on the frontend of the application interacting with the smart contract.

Remember, the best choice will depend on the specific needs and constraints of your application.

Assessed type

Other

[c4-judge]

0xean marked the issue as primary issue

[c4-sponsor]

chechu marked the issue as sponsor confirmed

[c4-judge]

0xean marked the issue as satisfactory

[c4-judge]

0xean marked the issue as selected for report

[Nabeel-javaid]

IMO this is a valid QA but not a medium. No loss for the protocol itself and is due to user mistake.

[rvierdiiev]

i would like to say that @Nabeel-javaid is right and this is user's mistake in order to participate in the auction user should know the rules

[0xean]

@Nabeel-javaid - thank you for flagging, after reviewing more, I do agree it should be QA as it does come down to input sanitization.

[c4-judge]

0xean changed the severity to QA (Quality Assurance)

[c4-judge]

0xean marked the issue as not selected for report