search

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

1 stars 0 forks source link

Basket can be initialized multiple times to completely take over the Basket contract
 #182

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

0xRajeev

Vulnerability details

Impact

Lack of OpenZeppelin's initializer modifier or a contract variable (as in Auction initialized tracking) to prevent multiple initializations using initialize() function allows anyone to initialize after contract deployment/initializing (from Factory) with arbitrary addresses/parameters, e.g. for factory/auction/publisher addresses, and completely take over contract management and funds.

Proof of Concept

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

Tools Used

Manual Analysis

Recommended Mitigation Steps

Allow initializing only once using an initialized bool logic similar to Auction contract.

GalloDaSballo commented 2 years ago

Agree with finding, while the change can look trivial, allowing initialize to be called multiple times allows for a hostile takeover from any external caller, as such this is a high severity finding

GalloDaSballo commented 2 years ago

Actually, after checking: #50 There is already an initializer check in __ERC20_init as such the initialze function cannot be called more than once

This means the exploit is not there, will downgrade to low and set #50 to be the main issue

GalloDaSballo commented 2 years ago

Duplicate of #50