Unprotected upgradeable contract

[hats-bug-reporter[bot]]

Github username: -- Twitter username: -- Submission hash (on-chain): 0x9333e73de86ac04bc09165f655705e5f1aeb6454f489799d3e55b5c478f01d5c Severity: medium

Description: Description\ Summary: Most contract is an upgradeable contract that does not protect its initialize functions. Anyone can delete the contract with: UUPSUpgradeable.upgradeToAndCall(address,bytes) (node_modules/@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol#92-95)

Detail: Most contract inherits from UUPSUpgradeable.sol and is deployed using a proxy pattern whereby the implementation contract is used by the proxy contract for all its logic. The proxy contract will make delegate calls to the implementation contract. This helps to facilitate future upgrades by pointing the proxy contract to a new and upgraded implementation contract.

However, if the implementation contract is left uninitialized, it is possible for any user to gain ownership of the onlyOwner role in the implementation contract for Most.sol. Once the user has ownership they are able to perform an upgrade of the implementation contract's logic contract and delegate call into any arbitrary contract, allowing them to alter the logic or self-destruct the proxy's implementation contract. Consequently, this will prevent all Most.sol interactions until a new implementation contract is deployed.

Initial information about this issue was found here.

Attack Scenario\ Developer deploys their contracts using their deployment scripts. These deployment scripts leave the implementation contracts uninitialized. Specifically the contract in question is Most.sol. This allows any arbitrary user to call initialize() on the Most.sol implementation contract. Once a user has gained control over Most.sol implementation contract, they can bypass the _authorizeUpgrade check used to restrict upgrades to the onlyOwner role. The malicious user then calls UUPSUpgradeable.upgradeToAndCall(). The new implementation contract then points to their own contract containing altered logic or self-destruct call in its fallback function. As a result, the implementation contract will have wrong logic or self-destructed due to the user-controlled delegate call, preventing all future calls to the Most.sol contract until a new implementation contract has been deployed.

Attachments

1. Proof of Concept (PoC) File file attached.

Issue details

File: contracts/Most.sol

/// @audit ******************* Issue Detail *******************
Most (contracts/Most.sol#15-375) is an upgradeable contract that does not protect its initialize functions: Most.initialize(address[],uint256,address,address) (contracts/Most.sol#83-97). Anyone can delete the contract with: UUPSUpgradeable.upgradeToAndCall(address,bytes) (node_modules/@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol#92-95)

/// @audit ************** Possible Issue Line(s) **************
    L#83-97,  

/// @audit ****************** Affected Code *******************
  83:     function initialize(
  84:         address[] calldata _committee,
  85:         uint256 _signatureThreshold,
  86:         address owner,
  87:         address payable _wethAddress
  88:     ) public initializer {
  89:         committeeId = 0;
  90:         wethAddress = _wethAddress;
  91: 
  92:         _setCommittee(_committee, _signatureThreshold);
  93: 
  94:         __Ownable_init(owner);
  95:         __Pausable_init();
  96:         _pause();
  97:     }

Files:

• PoC.sol (https://hats-backend-prod.herokuapp.com/v1/files/QmPae9y1YX8YPyAtwYfj5nChYJjypqinjcPYqM5anzRy9k)

[krzysztofziobro]

duplicate of https://github.com/hats-finance/Most--Aleph-Zero-Bridge-0xab7c1d45ae21e7133574746b2985c58e0ae2e61d/issues/4