search

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

0 stars 0 forks source link

nonReentrant missing in several commitEth functions #22

Closed code423n4 closed 3 years ago

code423n4 commented 3 years ago

Handle

gpersoon

Vulnerability details

Impact

The function commitEth of Crowdsale.sol is nonReentrant. However the commitEth functions of the other auction contracts don't have this keyword.

Assuming the nonReentrant is important, it should also be added in the other contracts

Proof of Concept

https://github.com/sushiswap/miso/blob/master/contracts/Auctions/Crowdsale.sol#L223 function commitEth(address payable _beneficiary, bool readAndAgreedToMarketParticipationAgreement ) public payable nonReentrant {

https://github.com/sushiswap/miso/blob/master/contracts/Auctions/BatchAuction.sol#L190 function commitEth(address payable _beneficiary, bool readAndAgreedToMarketParticipationAgreement) public payable {

https://github.com/sushiswap/miso/blob/master/contracts/Auctions/DutchAuction.sol#L261

function commitEth(address payable _beneficiary,bool readAndAgreedToMarketParticipationAgreement) public payable {

https://github.com/sushiswap/miso/blob/master/contracts/Auctions/HyperbolicAuction.sol#L265 function commitEth(address payable _beneficiary,bool readAndAgreedToMarketParticipationAgreement) public payable {

Tools Used

Recommended Mitigation Steps

Add nonReentrant to all the commitEth functions

Clearwood commented 3 years ago

We do not believe the functions to need this flag as at the end the virtual balance is compared to the actual balance

ghoul-sol commented 3 years ago

Given the that no exploit was actually found and sponsor comment I'm making this invalid.