-
Notifications
You must be signed in to change notification settings - Fork 982
Upgradeability Checks
slither-check-upgradability
is meant to help to review contracts using the delegatecall proxy pattern.
The tool checks that:
[This section is out of date]
slither-check-upgradeability proxy.sol ProxyName implem.sol ContractName
If a second version of the contract is to be checked, use:
slither-check-upgradeability proxy.sol ProxyName implem.sol ContractName implem_v2.sol ContractNameV2
proxy.sol
/implem.sol
can be a Truffle directory, for example:
slither-check-upgradeability . ProxyName . ContractName
If you use zos, you will have the proxy and the contract in different directories.
Likely, you will use one of the proxy from https://github.com/zeppelinos/zos. Clone the zos
, and install the dependencies:
git clone https://github.com/zeppelinos/zos
cd zos/packages/lib
npm install
rm contracts/mocks/WithConstructorImplementation.sol
Note: contracts/mocks/WithConstructorImplementation.sol
must be removed as it contains a name clash collision with contracts/mocks/Invalid.sol
Then from your project directory:
slither-check-upgradeability /path/to/zos/packages/lib/ UpgradeabilityProxy . ContractName
According to your setup, you might choose another proxy name than UpgradeabilityProxy
.
- Check:
became-constant
- Severity:
High
Detect state variables that should not be constant̀
.
contract Contract{
uint variable1;
uint variable2;
uint variable3;
}
contract ContractV2{
uint variable1;
uint constant variable2;
uint variable3;
}
Because variable2
is now a constant
, the storage location of variable3
will be different.
As a result, ContractV2
will have a corrupted storage layout.
Do not make an existing state variable constant
.
- Check:
function-id-collision
- Severity:
High
Detect function id collision between the contract and the proxy.
contract Contract{
function gsf() public {
// ...
}
}
contract Proxy{
function tgeo() public {
// ...
}
}
Proxy.tgeo()
and Contract.gsf()
have the same function id (0x67e43e43).
As a result, Proxy.tgeo()
will shadow Contract.gsf()`.
Rename the function. Avoid public functions in the proxy.
- Check:
function-shadowing
- Severity:
High
Detect function shadowing between the contract and the proxy.
contract Contract{
function get() public {
// ...
}
}
contract Proxy{
function get() public {
// ...
}
}
Proxy.get
will shadow any call to get()
. As a result get()
is never executed in the logic contract and cannot be updated.
Rename the function. Avoid public functions in the proxy.
- Check:
missing-calls
- Severity:
High
Detect missing calls to initialize functions.
contract Base{
function initialize() public{
///
}
}
contract Derived is Base{
function initialize() public{
///
}
}
Derived.initialize
does not call Base.initialize
leading the contract to not be correctly initialized.
Ensure all the initialize functions are reached by the most derived initialize function.
- Check:
missing-init-modifier
- Severity:
High
Detect if Initializable.initializer()
is called.
contract Contract{
function initialize() public{
///
}
}
initialize
should have the initializer
modifier to prevent someone from initializing the contract multiple times.
Use Initializable.initializer()
.
- Check:
multiple-calls
- Severity:
High
Detect multiple calls to a initialize function.
contract Base{
function initialize(uint) public{
///
}
}
contract Derived is Base{
function initialize(uint a, uint b) public{
initialize(a);
}
}
contract DerivedDerived is Derived{
function initialize() public{
initialize(0);
initialize(0, 1 );
}
}
Base.initialize(uint)
is called two times in DerivedDerived.initiliaze
execution, leading to a potential corruption.
Call only one time every initialize function.
- Check:
order-vars-contracts
- Severity:
High
Detect variables that are different between the original contract and the updated one.
contract Contract{
uint variable1;
}
contract ContractV2{
address variable1;
}
Contract
and ContractV2
do not have the same storage layout. As a result the storage of both contracts can be corrupted.
Respect the variable order of the original contract in the updated contract.
- Check:
order-vars-proxy
- Severity:
High
Detect variables that are different between the contract and the proxy.
contract Contract{
uint variable1;
}
contract Proxy{
address variable1;
}
Contract
and Proxy
do not have the same storage layout. As a result the storage of both contracts can be corrupted.
Avoid variables in the proxy. If a variable is in the proxy, ensure it has the same layout than in the contract.
- Check:
variables-initialized
- Severity:
High
Detect state variables that are initialized.
contract Contract{
uint variable = 10;
}
Using Contract
will the delegatecall proxy pattern will lead variable
to be 0 when called through the proxy.
Using initialize functions to write initial values in state variables.
- Check:
were-constant
- Severity:
High
Detect state variables that should be constant̀
.
contract Contract{
uint variable1;
uint constant variable2;
uint variable3;
}
contract ContractV2{
uint variable1;
uint variable2;
uint variable3;
}
Because variable2
is not anymore a constant
, the storage location of variable3
will be different.
As a result, ContractV2
will have a corrupted storage layout.
Do not remove constant
from a state variables during an update.
- Check:
extra-vars-proxy
- Severity:
Medium
Detect variables that are in the proxy and not in the contract.
contract Contract{
uint variable1;
}
contract Proxy{
uint variable1;
uint variable2;
}
Proxy
contains additional variables. A future update of Contract
is likely to corrupt the proxy.
Avoid variables in the proxy. If a variable is in the proxy, ensure it has the same layout than in the contract.
- Check:
missing-variables
- Severity:
Medium
Detect variables that were present in the original contracts but are not in the updated one.
contract V1{
uint variable1;
uint variable2;
}
contract V2{
uint variable1;
}
The new version, V2
does not contain variable1
.
If a new variable is added in an update of V2
, this variable will hold the latest value of variable2
and
will be corrupted.
Do not change the order of the state variables in the updated contract.
- Check:
extra-vars-v2
- Severity:
Informational
Show new variables in the updated contract.
This finding does not have an immediate security impact and is informative.
contract Contract{
uint variable1;
}
contract Proxy{
uint variable1;
uint variable2;
}
Proxy
contains additional variables. A future update of Contract
is likely to corrupt the proxy.
Ensure that all the new variables are expected.
- Check:
init-inherited
- Severity:
Informational
Detect if Initializable
is inherited.
Review manually the contract's initialization. Consider inheriting Initializable
.
- Check:
init-missing
- Severity:
Informational
Detect if a contract Initializable
is present.
Review manually the contract's initialization..
Consider using a Initializable
contract to follow standard practice.
- Check:
initialize_target
- Severity:
Informational
Show the function that must be called at deployment.
This finding does not have an immediate security impact and is informative.
Ensure that the function is called at deployment.
- Check:
initializer-missing
- Severity:
Informational
Detect the lack of Initializable.initializer()
modifier.
Review manually the contract's initialization. Consider inheriting a Initializable.initializer()
modifier.