Solmate's SafeTransferLib won't check if token has code, which can affect transfers in `SizeSealed`

[code423n4]

Lines of code

https://github.com/code-423n4/2022-11-size/blob/main/src/SizeSealed.sol#L163 https://github.com/code-423n4/2022-11-size/blob/main/src/SizeSealed.sol#L327 https://github.com/code-423n4/2022-11-size/blob/main/src/SizeSealed.sol#L351 https://github.com/code-423n4/2022-11-size/blob/main/src/SizeSealed.sol#L381 https://github.com/code-423n4/2022-11-size/blob/main/src/SizeSealed.sol#L439

Vulnerability details

Impact

Not checking for token existence is a know issue for Solmate. This can cause unexpected contract functionality for transfers implemented in SizeSealed. Note that this might not be a problem for baseToken due to the check implemented in L103. However, this can lead to issues for quoteToken.

Proof of Concept

• User calls createAuction() with a quoteToken in which the address doesn't contain code
• Calls to bid(), finalize(), refund(), withdraw() and cancelBid() will not revert and will result in silent failures.

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

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

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

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

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

Recommended Mitigation Steps

Check if baseToken, and most importantly if quoteToken, contain code when creating an auction.

diff --git a/src/SizeSealed.sol b/src/SizeSealed.sol
--- a/src/SizeSealed.sol
+++ b/src/SizeSealed.sol
@@ -81,6 +81,13 @@ contract SizeSealed is ISizeSealed {
             revert InvalidReserve();
         }

+        if (
+            auctionParams.baseToken.code.length == 0 ||
+            auctionParams.quoteToken.code.length == 0
+        ) {
+            revert InvalidToken();
+        }
+
         uint256 auctionId = ++currentAuctionId;

         Auction storage a = idToAuction[auctionId];

[trust1995]

Seller can theoretically suffer from abuse if they finalize an auction where malicious buyers bid with nonexisting tokens. However, only seller can be impacted negatively, and it requires serious negligence on their part.

[0xean]

Yea, there are some serious pre-conditions for this to occur, but could see it being M. Will leave open for sponsor review.

[c4-judge]

0xean marked the issue as primary issue

[midori-fuse]

However, only seller can be impacted negatively, and it requires serious negligence on their part.

Buyer can be impacted as well, if malicious sellers decides to make this an attack vector. See https://github.com/code-423n4/2022-11-size-findings/issues/48

[RagePit]

Incredibly unlikely but confirming as Medium given the good argument in #48

[c4-sponsor]

RagePit marked the issue as sponsor confirmed

[c4-judge]

0xean marked the issue as satisfactory

[C4-Staff]

captainmangoC4 marked issue #48 as primary and marked this issue as a duplicate of 48