code423n4 commented 2 years ago

Lines of code

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

Vulnerability details

Impact

Draining baseToken from SizeSealed contract by calling finalize function multiple times

Proof of Concept

The finalize() function can be called multiple times by providing clearingQuote to type(uint128).max.

Currently inside finalize() function there is no check condition if an auction already call finalize(). The only close check condition to this is the atState modifier which will prevent calling this finalize() again because it will move from RevealPeriod to Finalized state (if the clearingQuote is for some number (not type(uint128).max value)

File: SizeSealed.sol
217:     function finalize(uint256 auctionId, uint256[] memory bidIndices, uint128 clearingBase, uint128 clearingQuote)
218:         public
219:         atState(idToAuction[auctionId], States.RevealPeriod)
220:     {
221:         Auction storage a = idToAuction[auctionId];
222:         uint256 sellerPriv = a.data.privKey;
223:         if (sellerPriv == 0) {
224:             revert InvalidPrivateKey();
225:         }
226: 
227:         if (bidIndices.length != a.bids.length) {
228:             revert InvalidCalldata();
229:         }
230: 
231:         FinalizeData memory data;
232:         data.reserveQuotePerBase = a.params.reserveQuotePerBase;
233:         data.totalBaseAmount = a.params.totalBaseAmount;
234:         data.previousQuotePerBase = type(uint256).max;
235: 
236:         // Last filled bid is the clearing price
237:         a.data.lowestBase = clearingBase;
238:         a.data.lowestQuote = clearingQuote;
239: 
240:         // Bitmap of all the bid indices that have been processed
241:         uint256[] memory seenBidMap = new uint256[]((bidIndices.length/256)+1);
242: 
243:         // Fill orders from highest price to lowest price
244:         for (uint256 i; i < bidIndices.length; i++) {
245:             uint256 bidIndex = bidIndices[i];
246:             EncryptedBid storage b = a.bids[bidIndex];
247: 
248:             // Verify this bid index hasn't been seen before
249:             uint256 bitmapIndex = bidIndex / 256;
250:             uint256 bitMap = seenBidMap[bitmapIndex];
251:             uint256 indexBit = 1 << (bidIndex % 256);
252:             if (bitMap & indexBit == 1) revert InvalidState();
253:             seenBidMap[bitmapIndex] = bitMap | indexBit;
254: 
255:             // G^k1^k2 == G^k2^k1
256:             ECCMath.Point memory sharedPoint = ECCMath.ecMul(b.pubKey, sellerPriv);
257:             // If the bidder public key isn't on the bn128 curve
258:             if (sharedPoint.x == 1 && sharedPoint.y == 1) continue;
259: 
260:             bytes32 decryptedMessage = ECCMath.decryptMessage(sharedPoint, b.encryptedMessage);
261:             // If the bidder didn't faithfully submit commitment or pubkey
262:             // Or the bid was cancelled
263:             if (computeCommitment(decryptedMessage) != b.commitment) continue;
264: 
265:             // First 128 bits are the base amount, last are random salt
266:             uint128 baseAmount = uint128(uint256(decryptedMessage >> 128));
267: 
268:             // Require that bids are passed in descending price
269:             uint256 quotePerBase = FixedPointMathLib.mulDivDown(b.quoteAmount, type(uint128).max, baseAmount);
270:             if (quotePerBase >= data.previousQuotePerBase) {
271:                 // If last bid was the same price, make sure we filled the earliest bid first
272:                 if (quotePerBase == data.previousQuotePerBase) {
273:                     if (data.previousIndex > bidIndex) revert InvalidSorting();
274:                 } else {
275:                     revert InvalidSorting();
276:                 }
277:             }
278: 
279:             // Only fill if above reserve price
280:             if (quotePerBase < data.reserveQuotePerBase) continue;
281: 
282:             // Auction has been fully filled
283:             if (data.filledBase == data.totalBaseAmount) continue;
284: 
285:             data.previousQuotePerBase = quotePerBase;
286:             data.previousIndex = bidIndex;
287: 
288:             // Fill the remaining unfilled base amount
289:             if (data.filledBase + baseAmount > data.totalBaseAmount) {
290:                 baseAmount = data.totalBaseAmount - data.filledBase;
291:             }
292: 
293:             b.filledBaseAmount = baseAmount;
294:             data.filledBase += baseAmount;
295:         }
296: 
297:         if (data.previousQuotePerBase != FixedPointMathLib.mulDivDown(clearingQuote, type(uint128).max, clearingBase)) {
298:             revert InvalidCalldata();
299:         }
300: 
301:         // seenBidMap[0:len-1] should be full
302:         for (uint256 i; i < seenBidMap.length - 1; i++) {
303:             if (seenBidMap[i] != type(uint256).max) {
304:                 revert InvalidState();
305:             }
306:         }
307: 
308:         // seenBidMap[-1] should only have the last N bits set
309:         if (seenBidMap[seenBidMap.length - 1] != (1 << (bidIndices.length % 256)) - 1) {
310:             revert InvalidState();
311:         }
312: 
313:         if (data.filledBase > data.totalBaseAmount) {
314:             revert InvalidState();
315:         }
316: 
317:         // Transfer the left over baseToken
318:         if (data.totalBaseAmount != data.filledBase) {
319:             uint128 unsoldBase = data.totalBaseAmount - data.filledBase;
320:             a.params.totalBaseAmount = data.filledBase;
321:             SafeTransferLib.safeTransfer(ERC20(a.params.baseToken), a.data.seller, unsoldBase);
322:         }
323: 
324:         // Calculate quote amount based on clearing price
325:         uint256 filledQuote = FixedPointMathLib.mulDivDown(clearingQuote, data.filledBase, clearingBase);
326: 
327:         SafeTransferLib.safeTransfer(ERC20(a.params.quoteToken), a.data.seller, filledQuote);
328: 
329:         emit AuctionFinalized(auctionId, bidIndices, data.filledBase, filledQuote);
330:     }

if this call being executed multiple times it can drained the contract's baseToken

Tools Used

Manual analysis

Recommended Mitigation Steps

check condition if an auction is already called finalize() function, (perhaps by storing to the auction storage if it's being finalized), if so then revert

   Auction storage a = idToAuction[auctionId];
   if(a.finalized) revert AuctionAlreadyFinalized()
   else a.finalized = true

trust1995 commented 2 years ago

Warden spotted a weak spot in the contract, but did not specify a way to exploit it or provide a POC for it. Therefore, I believe it is unsatisfactory.

c4-judge commented 2 years ago

0xean marked the issue as duplicate