Lines of code

https://github.com/code-423n4/2024-07-basin/blob/7d5aacbb144d0ba0bc358dfde6e0cc913d25310e/src/WellUpgradeable.sol#L65 https://github.com/code-423n4/2024-07-basin/blob/7d5aacbb144d0ba0bc358dfde6e0cc913d25310e/src/WellUpgradeable.sol#L93 https://github.com/code-423n4/2024-07-basin/blob/7d5aacbb144d0ba0bc358dfde6e0cc913d25310e/src/WellUpgradeable.sol#L104

Vulnerability details

Impact

anyone can change the implementation contract due to no access control .

Proof of Concept

The WellUpgradeable contract inherits from UUPSUpgradeable and overrides the _authorizeUpgrade() function. However, without adding access control to this function, anyone can call it to set the implementation contract. This lack of restriction allows an attacker to set a malicious contract as the implementation, which can then use the selfdestruct() function to destroy the proxy contract. This is possible because the implementation is called using delegatecall from the proxy.

function _authorizeUpgrade(address newImplmentation) internal view override {

there two other functions are missing accesses control

function upgradeTo(address newImplementation) public override {
        _authorizeUpgrade(newImplementation);
        _upgradeToAndCallUUPS(newImplementation, new bytes(0), false);
    }

    /**
     * @notice Upgrades the implementation of the proxy to `newImplementation`.
     * Calls {_authorizeUpgrade}.
     * @dev `upgradeTo` was modified to support ERC-1167 minimal proxies
     * cloned (Bored) by an Aquifer.
     */
    function upgradeToAndCall(address newImplementation, bytes memory data) public payable override {
        _authorizeUpgrade(newImplementation);
        _upgradeToAndCallUUPS(newImplementation, data, true);
    }

Tools Used

Manual Review

Recommended Mitigation Steps

Add proper access control to _authorizeUpgrade() function since it is used for authorization at the time of changing implementation contract address. This can be mitigated by using 'OwnableUpgradeable' of openzeppelin and add onlyOwner modifier to the _authorizeUpgrade() function so. Only owner can be authorized to upgrade the implementation address.

Assessed type

Access Control

code-423n4 / 2024-07-basin-validation

anyone can change the implementation contract due to no access control . #64