code-423n4 / 2021-09-sushimiso-findings

0 stars 0 forks source link

Unchecked `fundsCommitted` in Token Withdrawal #74

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

leastwood

Vulnerability details

Impact

All auction contracts except BatchAuction.sol do not verify that fundsCommitted is greater than zero when executing withdrawTokens(). This helps to assert proper use of the function by users.

Proof of Concept

https://github.com/sushiswap/miso/blob/master/contracts/Auctions/HyperbolicAuction.sol#L466-L485 https://github.com/sushiswap/miso/blob/master/contracts/Auctions/Crowdsale.sol#L333-L349 https://github.com/sushiswap/miso/blob/master/contracts/Auctions/DutchAuction.sol#L499-L515

Tools Used

Manual code review

Recommended Mitigation Steps

Consider adding the line found in BatchAuction.sol:L331 to other auction contracts. The line is referenced here.

Clearwood commented 3 years ago

This is not a bug, as it will just lead to a useless call for users which will be prevented by the UI

ghoul-sol commented 3 years ago

making this a non-critical as it is best practices recommendation