An attacker can drain all funds in the protocol by taking over controller.sol because of insecure initialization.

Summary

The missing function "disableInitalizer" and a constructor to invoke it as recommended by Openzeppelin would allow an attacker to reinitialize the key roles in the protocol, then drain the protocol of all funds.

Root Cause

The failure to add function disable initializer and a constructor to invoke the _disableinitializer, makes the controller.sol contract vulnerable and by extension the entire protocol is vulnerable. After the first initialization, an attacker can reinitialize the contract by passing his address as the admin and gain control of the protocol. The protocol is compromised and the attacker would drain the pool. It can be seen in controller.sol:95 ,that the admin role and other important roles were initialized but not protected in the contract as recommended by Openzeppelin

Internal pre-conditions

No response

External pre-conditions

UserManager.sol needs to be called inorder to gain access to controller.sol which UserManger.sol is inheriting

Attack Path

The attacker sets up a malicious contract that would interact with the user contract called UserManager.sol
Through the interaction with UserManager.sol, the attacker can gain access to the internal function “controller init) in controller.sol because UserManager.sol inherits the controller.sol contract.
```
function __Controller_init(address admin_) internal onlyInitializing {
_paused = false;
admin = admin_;
__UUPSUpgradeable_init();
pauseGuardian = admin_;
}
```
Since this function has not been disabled using the function “disableinitializer” and a constructor to invoke it to ensure that the function is called only once during deployment, an attacker can easily reinitialize the protocol by passing his address as the admin. Therefore gaining control of the protocol.
The attacker can now drain the protocol of all funds.

Impact

The protocol suffers loss of all funds.

PoC

No response

Mitigation

MITIGATION You should invoke the {_disableInitializers} function in the constructor to automatically lock it when it is deployed as recommended by openzeppelin ![Uploading Screenshot from 2024-07-12 06-25-57.png…]()


 constructor() {
 *     _disableInitializers();
 * ```

sherlock-audit / 2024-06-union-finance-update-2-judging

0xpetern - An attacker can drain all funds in the protocol by taking over controller.sol because of insecure initialization. #135