Full report

zOS-lib audit

The audit was conducted on version 0.1.5, which can be found on GitHub.

Critical issues

No critical issues were found.

High severity issues

UpgradeabilityProxy is vulnerable to function selector clashing

zos-lib implements upgradable contracts using the Unstructured Storage Proxy Pattern. Like every other Proxy Pattern, it’s based on forwarding messages to an implementation contract using DELEGATECALL .

This forwarding logic is implemented in the Proxy ’s fallback function, which is only called if the current message’s data doesn’t start with one of the Proxy ’s functions’ selectors. This means that if the selector of one of the implementation’s functions clashes with one of the Proxy ’s, the latter will take precedence and the message will not be forwarded.

This behavior can possibly break contracts, but it can also be abused by an user of zos-lib to conceal malicious code. In the case of ZeppelinOS, this can be used for proposing backdoored library upgrades.

You can read our in-depth explanation of this vulnerability in this post.

Update: The Zeppelin team has already implemented a fix, which restricts the Proxy ’s functions only to its owner, forwarding every message that comes from other users to the implementation contract.

Medium severity issues

No medium severity issues were found.

Low severity issues

Initializable contracts do not work with multiple inheritance

If a contract has more than one Initializable direct parent, it will not be able to call both initializers due to the handling of the initialized variable.

Update: The Zeppelin team was already aware of this issue, and they’re planning to implement a new initialization mechanism soon. In the meantime, documentation about this has been added.

ContractDirectory#setImplementation doesn’t check if the implementation is a contract

This function can be called with any valid address, including the null address and externally owned accounts, which can lead to a corrupted ContractDirectory .

Update: Fixed in pull request #102.

Migrations may be applied in arbitrary order

Migratable is used to implement upgradable contracts’ state updates in-between versions. It lets developers implement migrations that can require for the execution of another specific migration to have happened first.

If two migrations depend on the same one they can be applied in any order. This can corrupt the contract’s state, as the first migration may have altered it in ways unpredicted by the second migration’s author.

Update: The Zeppelin team is planning to implement a new initialization mechanism soon. In the meantime, documentation about this has been added.

BaseAppManager#upgradeTo and BaseAppManager#upgradeToAndCall can be called with an incorrect contract name

These methods are used to upgrade proxy contracts and take as inputs the name of the implementation these should be upgraded to. This name is never checked to be correct, and if it isn’t, the contract will almost certainly be corrupted.

Update: This functions are intended to be called using zOS’s CLI, which validates this. Future on-chain validation is being evaluated.

Comments and recommendations