UNRESTRICTED USAGE OF CREATELOT() RISKS UNINTENTIONAL SD TOKEN DONATIONS

[code423n4]

Lines of code

https://github.com/code-423n4/2023-06-stader/blob/main/contracts/Auction.sol#L48-L60

Vulnerability details

Impact

Auction.createLot allows unrestricted access. Consequently, any user can call this function, potentially leading to unintended donations of their Stader tokens. If no bidder emerges at the completion of an auction, these tokens are transferred to the treasury on line 114:

Auction.sol#L106-L118

    function extractNonBidSD(uint256 lotId) external override {
        LotItem storage lotItem = lots[lotId];
        if (block.number <= lotItem.endBlock) revert AuctionNotEnded();
        if (lotItem.highestBidAmount > 0) revert LotWasAuctioned();
        if (lotItem.sdAmount == 0) revert AlreadyClaimed();

        uint256 _sdAmount = lotItem.sdAmount;
        lotItem.sdAmount = 0;
114:        if (!IERC20(staderConfig.getStaderToken()).transfer(staderConfig.getStaderTreasury(), _sdAmount)) {
            revert SDTransferFailed();
        }
        emit UnsuccessfulSDAuctionExtracted(lotId, _sdAmount, staderConfig.getStaderTreasury());
    }

Else, the ETH proceeds go to the stake pool manager on line 102:

Auction.sol#L93-L104

    function transferHighestBidToSSPM(uint256 lotId) external override nonReentrant {
        LotItem storage lotItem = lots[lotId];
        uint256 ethAmount = lotItem.highestBidAmount;

        if (block.number <= lotItem.endBlock) revert AuctionNotEnded();
        if (ethAmount == 0) revert NoBidPlaced();
        if (lotItem.ethExtracted) revert AlreadyClaimed();

        lotItem.ethExtracted = true;
102:        IStaderStakePoolManager(staderConfig.getStakePoolManager()).receiveEthFromAuction{value: ethAmount}();
        emit ETHClaimed(lotId, staderConfig.getStakePoolManager(), ethAmount);
    }

Proof of Concept

createLot() is accessible to all. Someone may create a phishing link that approves the SD token transfer and trick unaware users calling the function:

Auction.sol#L48

    function createLot(uint256 _sdAmount) external override whenNotPaused {

This function is supposed to be called by SDCollateral.slashSD for sending the slashed amount to auction. Regrettably, the Auction contract lacks restrictions that would prevent arbitrary users from invoking createLot() outside of a slashing context.

Recommended Mitigation Steps

It is suggested restricting createLot to only the SDCollateral contract to avoid users` accidental calls.

Assessed type

Access Control

[c4-judge]

Picodes marked the issue as duplicate of #106

[c4-judge]

Picodes changed the severity to QA (Quality Assurance)

[c4-judge]

Picodes marked the issue as grade-b

[Picodes]

Giving unsatisfactory to this submission as it is with sufficient likelihood a direct copy of #106

[c4-judge]

Picodes marked the issue as grade-c