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

0 stars 0 forks source link

Missing Access Control in `upgradeTo` Function #68

Closed c4-bot-3 closed 3 months ago

c4-bot-3 commented 3 months ago

Lines of code

https://github.com/code-423n4/2024-07-basin/blob/main/src/WellUpgradeable.sol#L93-L96

Vulnerability details

Contract Code: WellUpgradeable.sol#L93-L96

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

function upgradeToAndCall(address newImplementation, bytes memory data) public payable override {
        _authorizeUpgrade(newImplementation);
        _upgradeToAndCallUUPS(newImplementation, data, true);
    }

Issue Summary: The upgradeTo and upgradeToAndCall function lacks proper access control modifiers, which may allow unauthorized users to perform an upgrade operation. This poses a significant security risk as it enables anyone to upgrade the contract implementation, potentially leading to malicious activities or unintended behaviors.

Detailed Description:

  1. Function Purpose:

    • The upgradeTo function is intended to upgrade the contract's implementation to a new address (newImplementation). It calls the internal function _authorizeUpgrade to verify authorization and then performs the upgrade using _upgradeToAndCallUUPS.
  2. Missing Access Control:

    • The function is marked as public, meaning it can be called by any address. Without proper access control, any user can trigger the upgrade process, which can lead to:
      • Unauthorized upgrades to the contract implementation.
      • Potential introduction of malicious code or vulnerabilities in the contract.

Additional Notes:

Tools Used

Manual review

Recommended Mitigation Steps

import "@openzeppelin/contracts/access/Ownable.sol";

contract MyContract is Ownable {
    function upgradeTo(address newImplementation) public override onlyOwner {
        _authorizeUpgrade(newImplementation);
        _upgradeToAndCallUUPS(newImplementation, new bytes(0), false);
    }
}

Assessed type

Access Control