code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Impact

With updateBidIncrement it will update main bidIncrement value for every auction. Technically when it would trigger It will cause unbalanced auctions for customers.

    function updateBidIncrement(uint256 _bidIncrement) external override {
        UtilLib.onlyManagerRole(msg.sender, staderConfig);
        bidIncrement = _bidIncrement;
        emit BidIncrementUpdated(_bidIncrement);
    }

In below code, the contract checks for bidIncrement amount and compare it to totalUserBid. once a bid placed manager could change that bidIncrement and cause unbalanced result. For example, manager could enter big amount to block other new bidders from to entering the auction.

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

Tools Used

Manual review

Recommended Mitigation Steps

Either it should be stored in the lotItem struct for bidIncrement amount like:

    struct LotItem {
        uint256 startBlock;
        uint256 endBlock;
        uint256 sdAmount;
        mapping(address => uint256) bids;
        address highestBidder;
        uint256 highestBidAmount;
        uint256 bidIncrement;
        bool sdClaimed;
        bool ethExtracted;
    }

and use this value for checking in addBid function like:

    function addBid(uint256 lotId) external payable override whenNotPaused {
        // reject payments of 0 ETH
        if (msg.value == 0) revert InSufficientETH();

        LotItem storage lotItem = lots[lotId];
        if (block.number > lotItem.endBlock) revert AuctionEnded();

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

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

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

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

or updateBidIncrement function must check all ongoing bids.

Assessed type

Other

c4-judge commented 1 year ago

Picodes changed the severity to QA (Quality Assurance)