Unguarded Initializer
Briefing
Contracts behind a proxy cannot use constructors to set state, because the constructor runs in the implementation’s storage, not the proxy’s. Initialisation moves into an ordinary function that must run exactly once.
The team deploys the proxy in one transaction and calls `initialize` in a second transaction from their deployment script.
// Quantix Security Research Network — training challenge.// Intentionally vulnerable. Written for education; do not deploy.// SPDX-License-Identifier: MITpragma solidity 0.8.24; interface IERC20 { function transfer(address to, uint256 amount) external returns (bool);} contract RewardsDistributorV1 { address public owner; address public rewardToken; bool private initialized; function initialize(address _owner, address _rewardToken) external { require(!initialized, "already initialized"); owner = _owner; rewardToken = _rewardToken; initialized = true; } function sweep(address token, address to, uint256 amount) external { require(msg.sender == owner, "not owner"); require(IERC20(token).transfer(to, amount), "transfer failed"); }}// Transaction 1: deploy implementation and proxy without init dataRewardsDistributorV1 impl = new RewardsDistributorV1();ERC1967Proxy proxy = new ERC1967Proxy(address(impl), ""); // Transaction 2: initialise from the deployerRewardsDistributorV1(address(proxy)).initialize(TEAM_MULTISIG, REWARD_TOKEN);Hints
Reveal one at a time. Each moves a step closer to the answer.
- H1
Hidden
- H2
Hidden
- H3
Hidden
Solution
Try writing your own explanation and fix before opening it.
`initialize` has no access control: whoever calls it first becomes `owner`. Because the proxy is deployed without initialisation data, there is a window between the two transactions in which any account can call `initialize` on the proxy and set itself as owner. The team’s own call then reverts with "already initialized", and the attacker can call `sweep` once rewards are funded.
The bare implementation is also never initialised, so anyone can take ownership of it. For this contract that is harmless, but in UUPS designs an owned implementation can be used to call upgrade functions on the implementation itself.
Fix both: initialise atomically by passing the encoded `initialize` call as the proxy constructor’s data, and disable initialisers on the implementation in its constructor.
-contract RewardsDistributorV1 {- address public owner;- address public rewardToken;- bool private initialized;-- function initialize(address _owner, address _rewardToken) external {- require(!initialized, "already initialized");+import {Initializable} from "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";++contract RewardsDistributorV1 is Initializable {+ address public owner;+ address public rewardToken;++ /// @custom:oz-upgrades-unsafe-allow constructor+ constructor() {+ _disableInitializers();+ }++ function initialize(address _owner, address _rewardToken) external initializer { owner = _owner; rewardToken = _rewardToken;- initialized = true; } // deploy.s.sol-ERC1967Proxy proxy = new ERC1967Proxy(address(impl), "");-RewardsDistributorV1(address(proxy)).initialize(TEAM_MULTISIG, REWARD_TOKEN);+ERC1967Proxy proxy = new ERC1967Proxy(+ address(impl),+ abi.encodeCall(RewardsDistributorV1.initialize, (TEAM_MULTISIG, REWARD_TOKEN))+);