Phoenix-Protocol-Group / phoenix-contracts

Source code of the smart contracts of the Phoenix DeFi hub DEX protocol
GNU General Public License v3.0
10 stars 6 forks source link

PHOAM-018: Attackers can take over the vesting contract #349

Closed gangov closed 1 month ago

gangov commented 2 months ago

Location

./contracts/vesting/src/contract.rs:79

Description Anyone can re-initialize the vesting contract to gain administrator privileges and perform various privileged tasks, such as updating the minter, creating schedules, and updating the contract's implementation. The initialize function does not restrict the number of times it can be called. Additionally, it does not verify that the admin address passed as a parameter matches the existing contract administrator address, if one is set.

By passing their address as the admin parameter, an adversary can gain privileged access to the contract and carry out the aforementioned tasks. The same scenario happens with the initialize_with_minter() function, that has no checks to prevent re-initialization.

Recommendation Ensure the initialize and initialize_with_minter() functions can only be called once, similar to the pool contracts.

Alternatively, ensure that only the contract administrator can call this function. Consider initializing the contract immediately after its deployment (atomically). This may require deploying the trader contract from a factory contract. Additionally, add adversarial tests to the testing suites to identify authorization issues.