SIZE contest - RedOneN's results

Lines of code

https://github.com/code-423n4/2022-11-size/blob/main/src/SizeSealed.sol#L157-L159

Vulnerability details

Impact

SIZE contract does not prevent the same bidder to submit multiple bids from the same address or from multiple different addresses. However, to prevent DOS attacks, SIZE introduces a condition on the total number of bids that could take place for an auction (max 1000 bids). Although such condition prevents DOS and allows the "Finalize()" function to have a "finite" for loop, it opens the door to another another risk. By limiting the total number of bid to 1000, it gives the possibility to a malicious bidder to perform 1000 transactions at the "minimumBidQuote" under three scenarios :

1. Executing exactly the same bid 1000 times. If the reserveQuotePerBase is reached then each of the 1000 transactions will be executed at the attackers' desired price.
2. Executing 1000 transactions with a QuotePerBase that starts very low and gradually increases. By doing so, the attackers ensures that he will reach the reserveQuotePerBase. All transactions below the reserveQuotePerBase will be refunded to the attackers while other transactions will be executed at the most advantageous price (limit set by the seller).
3. Executing 1000 transactions with a ridiculously low QuotePerBase. This will only block the auction with no "economic benefit" for the attackers that will just reclaim it's token.

Note that that such attack could very easily take place if the "minimumBidQuote" value is low. E.g. if we assume USDC being the "QuoteToken" and a minimumBidQuote of 1 USDC, any bidder with a balance of at least 1000USDC could perform the attack.

As a result, the protocol deviates from its main objective since :

1. A bidder is able the win the auction at the most advantageous price (limit set by the seller).
2. The "opportunity loss" is supported by the seller.
3. Fair bidders are excluded from the auction. At best, they are able to bid and would benefit from a partial attack performed by malicious bidder under scenario 2.

Proof of Concept

See link provided above.

Tools Used

Manual audit .

Recommended Mitigation Steps

Possibilities to mitigate this risk includes :

1. Increase the maximum number of bids allowed per auction while keeping the impact on the "finalize()" function under control. This would increase the balance needed to perform the attack.
2. Increase the "minimumBidQuote" such that the balance needed to perform 1000 transactions is much higher.
3. Set a maximum number of bid per wallet.
4. Impose a whitelist for each auction.

[L-01] Check that Contract Exists before using solmate's SafeTransferLib

Functions in solmate's SafeTransferLib library do not check whether a token has code at all. This responsibility is delegated to the caller. As a call to an address with no code will be a no-op, since low-level calls to non-contracts always return true, a transfer of tokens using solmate's SafeTransferLib will succeed if the token does not have any code. Therefore, it is recommended to verify that a contract exists before using any SafeTransferLib functions.

In the case of SIZE, this check is indirectly performed for the "baseToken" by verifying that the contract balance before and after transfer is coherent. However, no check is performed for the "quoteToken". As a result, Seller might accidentally provide a non-existing address. As a result, he will create an auction and pay GAS for an auction that no one Wille participate in.

[L-02] Bidders are still allowed to bid after a Seller canceled the auction.

During the life of an auction, a seller is allowed to cancel an auction as long as the auction is not over (block.timestamp < EndDate). If so, the contract logic set the EndDate to "infinite" so that the auction never end. Doing so does not induce any trouble in term of fund retrieve (bidders and seller are able to claim back their fund) but it does not prevent new bidders to bid on this canceled auction and pay GAS.

[L-03] No time limit on "vestingEndDate".

SIZE contract provides the seller with the possibility to include a vesting period. However, there is no limit set on the vesting end date. As a result, a seller could set an infinite vesting end date. A bidder that did not pay attention at the vesting time will only be able to claim the cliff amount.

[N-01] Constants should be defined rather than using magic numbers.

Even assembly can benefit from using readable constants instead of hex/numeric literals.

File : SizeSealed.sol

SizeSealed.sol#L35 SizeSealed.sol#L37 SizeSealed.sol#L241 SizeSealed.sol#L249 SizeSealed.sol#L266 SizeSealed.sol#L426

[N-02] Use of block.timestamp

Block timestamps have historically been used for a variety of applications, such as entropy for random numbers (see the Entropy Illusion for further details), locking funds for periods of time, and various state-changing conditional statements that are time-dependent. Miners have the ability to adjust timestamps slightly, which can prove to be dangerous if block timestamps are used incorrectly in smart contracts.

File : CommonTokenMath.sol

CommonTokenMath.sol#L40 CommonTokenMath.sol#L41

File : SizeSealed.sol

SizeSealed.sol#L29 SizeSealed.sol#L31 SizeSealed.sol#L35 SizeSealed.sol#L37 SizeSealed.sol#L60 SizeSealed.sol#L425 SizeSealed.sol#L426 SizeSealed.sol#L460

[N-03] Incomplete NatSpec

Details about some important parameters would be useful :

https://github.com/code-423n4/2022-11-size/blob/main/src/interfaces/ISizeSealed.sol#L63-L68.