code-423n4 / 2022-11-size-findings

1 stars 0 forks source link

baseToken amount is not contained per Auction can messed up vesting accounting #294

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

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

Vulnerability details

Impact

baseToken amount is not contained per Auction can messed up vesting accounting

Proof of Concept

If there are multiple auction with using same baseToken (which this is possible), there is probability accounting of baseToken is going to messed up because of one incident.

For example the incident which were high severity issue I submitted (seller create auction -> bidder bids -> reveal (with no finalizeData) -> call finalize, and set (clearingQuote) lowestQuote to type(uint128).max -> call cancelAuction) this seller doesn't stop, because the seller recalling finalize() function with correct clearingQuote therefore the state now is correctly Finalized so bidder can claim their vesting.

if we rethink again, based on this scenario, the first finalize() call, will return the seller's baseToken when he createAuction. but the second finalize() is using the baseToken which is not from the sellers auction but from other auction with the same baseToken.

This makes accounting of baseToken for auction will corrupted.

Tools Used

Manual analysis

Recommended Mitigation Steps

Contain/separate the baseToken for each auction, maybe different vault per auction.

trust1995 commented 2 years ago

Very vague and no actual issue demonstrated. baseToken is chosen per auction.

0xean commented 2 years ago

warden uses already submitted issue as proof that the issue can lead to bad outcomes... downgrade this to QA, as the segregation of auction funds is a reasonable suggestion but doesn't stand alone

c4-judge commented 2 years ago

Duplicate of https://github.com/code-423n4/2022-11-size-findings/issues/267