search

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

1 stars 0 forks source link

Missing check for auctionOngoing is risky #157

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Handle

0xRajeev

Vulnerability details

Impact

killAuction() is missing a require() to check that auctionOngoing == true before setting it to false. While currently, the caller publishNewIndex() in Basket has this condition checked, any other usages may accidentally call this when auction is not ongoing.

Proof of Concept

https://github.com/code-423n4/2021-09-defiProtocol/blob/52b74824c42acbcd64248f68c40128fe3a82caf6/contracts/contracts/Auction.sol#L43-L45

https://github.com/code-423n4/2021-09-defiProtocol/blob/52b74824c42acbcd64248f68c40128fe3a82caf6/contracts/contracts/Basket.sol#L175-L187

Tools Used

Manual Analysis

Recommended Mitigation Steps

Add require(auctionOngoing == true)

GalloDaSballo commented 2 years ago

killAuction can only be called by Basket, Basket will call it only in the else condition of publishNewIndex

That means that it will be called exclusively if !auction.auctionOngoing() == false

While the advice is sound for the function, it bears no impact on the broader system

Will change severity to non-critical