MultiSig Timelock
First Flight #55
Beginner FriendlyWallet
100 EXP
View all submissions
Previous Next
Submission Details
Impact: medium
Likelihood: medium

Missing @custom:oz-upgrades-unsafe-allow for Upgradable Contracts

Author Revealed upon completion

Root + Impact

Description

  • Missing @custom:oz-upgrades-unsafe-allow for Upgradable Contracts

  • If the contract is intended to be upgradable (e.g., using OpenZeppelin's TransparentUpgradeableProxy), the @custom:oz-upgrades-unsafe-allow directive is missing for functions that modify storage layout (e.g., grantSigningRole).

// @custom:oz-upgrades-unsafe-allow

Risk

Likelihood:

  • The grantSigningRole function modifies the s_signers array and s_signerCount state variables.

  • Without the @custom:oz-upgrades-unsafe-allow directive, upgrading the contract could lead to storage layout conflicts or unexpected behavior.

Impact:

  • Deployment Failure: If the contract is deployed as an upgradable proxy, the absence of the directive may cause the upgrade to fail or behave unpredictably.

  • Storage Corruption: Upgrades could overwrite or misalign storage variables, leading to data loss or contract instability.

Proof of Concept

simulate an upgrade to the MultiSigTimelock contract. The original contract modifies storage (s_signers, s_signerCount), but lacks the @custom:oz-upgrades-unsafe-allow directive. When upgraded, the proxy will fail to map storage correctly.

//original contract
// @custom:oz-upgrades-unsafe-allow state-variable-immutable
pragma solidity ^0.8.19;
​
import "@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol";
import "@openzeppelin/contracts/access/Ownable.sol";
​
contract MultiSigTimelockV1 is Ownable {
address[5] private s_signers;
uint256 private s_signerCount;
​
function grantSigningRole(address _account) external onlyOwner {
s_signers[s_signerCount] = _account;
s_signerCount += 1;
}
}
​
​
// Upgraded Contract (Modified Storage Layout)
// @custom:oz-upgrades-unsafe-allow state-variable-immutable
pragma solidity ^0.8.19;
​
import "@openzeppelin/contracts/access/Ownable.sol";
​
contract MultiSigTimelockV2 is Ownable {
address[5] private s_signers;
uint256 private s_signerCount;
uint256 private newVariable; // New storage slot added
​
function grantSigningRole(address _account) external onlyOwner {
s_signers[s_signerCount] = _account;
s_signerCount += 1;
}
}
​
// Deploy the original contract as a proxy
MultiSigTimelockV1 original = new MultiSigTimelockV1();
TransparentUpgradeableProxy proxy = new TransparentUpgradeableProxy(
address(original), admin, abi.encodeWithSelector(original.grantSigningRole.selector, address(0x1))
);
​
// Attempt to upgrade to V2 (missing @custom:oz-upgrades-unsafe-allow)
MultiSigTimelockV2 upgraded = new MultiSigTimelockV2();
proxy.upgradeTo(address(upgraded));

Recommended Mitigation

Add the @custom:oz-upgrades-unsafe-allow directive to the contract:


​
+ add this code
// @custom:oz-upgrades-unsafe-allow state-variable-immutable
contract MultiSigTimelock is Ownable, AccessControl, ReentrancyGuard {
// ... rest of the contract
}
​
​
​

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!