code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-06-stader/blob/7566b5a35f32ebd55d3578b8bd05c038feb7d9cc/contracts/Auction.sol#L71 https://github.com/code-423n4/2023-06-stader/blob/7566b5a35f32ebd55d3578b8bd05c038feb7d9cc/contracts/Auction.sol#L48-L78

Vulnerability details

medium

Links: https://github.com/code-423n4/2023-06-stader/blob/7566b5a35f32ebd55d3578b8bd05c038feb7d9cc/contracts/Auction.sol#L71

https://github.com/code-423n4/2023-06-stader/blob/7566b5a35f32ebd55d3578b8bd05c038feb7d9cc/contracts/Auction.sol#L48-L78

Title: SD Collateral Auction can be gamed

Impact

The SD Collateral Auction can be easily exploited.

In the auction contract, the "addBid" function uses a fixed value called "MinBidIncrement" to verify the bid amount, regardless of the number of tokens being auctioned.

This allows someone to sell a large amount of SD Collateral for a very small price if they are the only participant in the auction. Since the SD Collateral is usually worth 0.2 to 4 ETH, selling it at a low price would lead to significant losses.

Proof of Concept

In addBid() of the Auction contract, we only check if the totalUserBid is larger then the highestBidAmount + bidIncrement:

uint256 totalUserBid = lotItem.bids[msg.sender] + msg.value;

if (totalUserBid < lotItem.highestBidAmount + bidIncrement) revert InSufficientBid(); // could be as little as bidIncrement

lotItem.highestBidder = msg.sender;
lotItem.highestBidAmount = totalUserBid;
lotItem.bids[msg.sender] = totalUserBid;

emit BidPlaced(lotId, msg.sender, totalUserBid);

https://github.com/code-423n4/2023-06-stader/blob/7566b5a35f32ebd55d3578b8bd05c038feb7d9cc/contracts/Auction.sol#L69-L77

When there are many SD tokens being auctioned and multiple auctions happening simultaneously, the SD collateral could be sold for much less than its true worth. This could lead to a significant loss for the protocol.

when user create an auction

the supplied parameter is the amount of SD that is in the auction

function createLot(uint256 _sdAmount) external override whenNotPaused {

user can use ETH to bid for SD amount

if let us say the minIncrement amount is 0.1 ETH

if (totalUserBid < lotItem.highestBidAmount + bidIncrement) revert InSufficientBid();

if user pay 1 ETH

the next user has to pay 1.1 ETH

next user needs to pay 1.2 ETH to win the auction

the bidIncrement has no connection between the amount of SD token that is in auction

assume the there are 1 million SD in auction which worth 10 ETH, and no one bother to do the auction

a user can use 0.1 ETH (minIncrement) to buy the 1 million SD token in the last minutes

free profit for user and large loss for the protocol

there are likely ten or twenty auction that is online at the same time one single bidIncrement cannot fit them all

Tools Used

VS Code

Recommended Mitigation Steps

ncorporate SD token price oracle to make sure the highest bid is not so cheap and unreasonable when the auction is settled

Assessed type

Token-Transfer

c4-judge commented 1 year ago

Picodes changed the severity to QA (Quality Assurance)

Picodes commented 1 year ago

Downgrading to QA as this report only highlight that the market has to be efficient