code423n4 commented 3 years ago

Handle

0xsanson

Vulnerability details

Impact

The owner of Factory contract can modify the values of auctionMultiplier and auctionDecrement at any time. During an auction, these values are used to calculate newRatio and thereby tokensNeeded: specifically, it's easy to set the factory parameters so that tokensNeeded = 0 (or close to zero) for every token. This way the owner can participate at an auction, change the parameters, and get the underlying tokens from a Basket without transferring any pending tokens.

Proof of Concept

https://github.com/code-423n4/2021-09-defiProtocol/blob/main/contracts/contracts/Auction.sol#L89-L99

Tools Used

editor

Recommended Mitigation Steps

Consider adding a Timelock to these Factory functions. Otherwise a way to not modify them if an auction is ongoing (maybe Auction saves the values it reads when startAuction is called).

frank-beard commented 3 years ago

the owner for this is intended to be a dao that acts in support of the protocol, however this is a good point to the centralization concerns for the protocol, we will most likely manage this by adding a timelock to these function

GalloDaSballo commented 2 years ago

Agree with the finding from the warden, highly recommend the sponsor to add these possibilities in their docs to make it easy for users to understand them.

That said, a possible remediation would also be to add checks in the setters, to avoid for these specific edge cases.

Because the exploit is dependent on an external condition (setting to a specific value by governance), the finding is of medium severity

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