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

0 stars 0 forks source link

No access control on initialise() will lead to stolen funds #169

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

0xwags

Vulnerability details

Impact

In Auction.initialize() can be called by anyone and after such call, initialise will be 'true' preventing the intended parties from setting an address for 'basket'. Furthermore, an attacker can take control of key functions within the protocol from a controlled address such as startAuction(), killAuction()

Proof of Concept

Attacker can start the auction from the address used then wait for auction period and collect funds for eg, in bond forRebalance() on lines 67,the bondAmount from the user in the external call to safetransferFrom() will be sent to the malicious 'address(basket)' . Also in settleAuction() lines 89,93 & 107

Tools Used

Manual Analysis

Recommended Mitigation Steps

Recommend using proper access controls on initialize()

frank-beard commented 2 years ago

Initialize is called when the basket and auction contracts are created in the createBasket method, and they can only be called once, so this is not a valid issue.

0xleastwood commented 2 years ago

Auction.initialize() contains an initialized variable which prevents multiple calls to this function. As such, this is an invalid issue.