LayerZero smart contract audit report

ABSTRACT

Dedaub was commissioned to perform a security audit of LayerZero DVN-AVS contracts. The audit covered the implementation of LayerZero on-chain DVN-AVS based on EigenLayer middleware. Overall, the audit identified 1 High, 1 Medium, and 1 Low issues that were all resolved by the developers during the audit.The High and Medium issue are related to the admin role for the AVS which removed from the protocol as a fix for these issues.

BACKGROUND

LayerZero DVN-AVS integrates Layer Zero’s Data Verification Node (DVN) system with EigenLayer’s Autonomous Verifiable Service (AVS) infrastructure, implementing Eigenlayer middleware contracts to enable operator registration and slashing. It involves a registry coordinator that allows the registration of a single operator that cannot be deregistered. Additionally, it implements a bonded slashing mechanism to slash the operator if he violates the protocol rules. Anyone can act as a challenger and submit slashing requests. The approval or cancellation of the slashing request is managed by a privileged address representing a security council. The security council role can be transferred in a two step process where the current security council can initiate the transfer and the new security council can accept the transfer of the role within a defined time period.

SETTING & CAVEATS

This audit report mainly covers the contracts of the (at-the-time private) repository https://github.com/Layr-Labs/DVN-AVS of the Protocol LayerZero DVN-AVS at commit 0aee8535d46b517a8c5d8f9f6fb8074b9596e1da.

Two auditors worked on the codebase for 3 days on the following contracts:

src//
├── eigenlayer/
│   ├── LayerZeroDVNSlasher.sol
│   ├── LayerZeroRegistryCoordinator.sol
│   ├── LayerZeroServiceManager.sol
│   └── TimeboundSlasher.sol
└── interfaces/
    ├── ILayerZeroDVNSlasher.sol
    ├── ILayerZeroRegistryCoordinator.sol
    └── ITimeboundSlasher.sol

The audit’s main target is security threats, i.e., what the community understanding would likely call “hacking”, rather than the regular use of the protocol. Functional correctness (i.e. issues in “regular use”) is a secondary consideration. Typically it can only be covered if we are provided with unambiguous (i.e. full-detail) specifications of what is the expected, correct behavior. In terms of functional correctness, we often trusted the code’s calculations and interactions, in the absence of any other specification. Functional correctness relative to low-level calculations (including units, scaling and quantities returned from external protocols) is generally most effectively done through thorough testing rather than human auditing.

VULNERABILITIES & FUNCTIONAL ISSUES

This section details issues affecting the functionality of the contract. Dedaub generally categorizes issues according to the following severities, but may also take other considerations into account such as impact or difficulty in exploitation:

Issue resolution includes “dismissed” or “acknowledged” but no action taken, by the client, or “resolved”, per the auditors.

CRITICAL SEVERITY

    [No critical severity issues]

HIGH SEVERITY

Admin of the AVS can perform immediate slashes

High | Status: RESOLVED

The protocol defines a security council, typically an account with multisig access from trusted entities, which acts as the authority to approve operator slashes. Additionally, the documentation states the following: “The Security council is the only entity authorized to accept a slashing request”. This mechanism is used to ensure less centralization and reduce the trust assumption of the DVN-AVS.

However, the contract LayerZeroServiceManager provides a functionality to give administrative privilege to one or more admin accounts using IPermissionController. Once an admin account is set, the admin is permitted to call the function IAllocationManager::slashOperator() directly on behalf of the LayerZero DVN-AVS enabling them to perform immediate slashes to operators bypassing the security council approval. Therefore, giving the admin the permission to perform slashes defeats the purpose of having a trusted security council.

A proof of concept is provided in Proof of Concept - H1.

MEDIUM SEVERITY

LayerZeroServiceManager cannot be used after setting an admin

Medium | Status: RESOLVED

The owner of the contract LayerZeroServiceManager can set an admin by calling LayerZeroServiceManager::addPendingAdmin() and then the admin accepting using permissionController.acceptAdmin(). However, the permissionController contract only allows the account (i.e., LayerZeroServiceManager contract) to call its functions when an admin is not yet set. Once the admin is set, the permissionController functions are only callable by the admin. Therefore, once an admin is set, the following functions of LayerZeroServiceManager contract cannot be called by anyone:

LayerZeroServiceManager::addPendingAdmin()
LayerZeroServiceManager::removePendingAdmin()
LayerZeroServiceManager::removeAdmin()
LayerZeroServiceManager::setAppointee()
LayerZeroServiceManager::removeAppointee()

Therefore, any further updates for the permission of BVN-AVS can be only done by the admin by directly calling the permissionController functions and not through LayerZeroServiceManager functions. For example, the function LayerZeroServiceManager::removeAdmin() can never be called because once an admin is added, this function is disabled.

If the intention is to allow the owner to keep the rights of updating permission, then one should consider adding the BVN-AVS contract as an admin to itself by adding in the constructor of LayerZeroServiceManager the following calls:

permissionController.addPendingAdmin(address(this), address(this)) permissionController.acceptAdmin(address(this))

LOW SEVERITY

Unsafe calls to external ERC20 bond token contract

Low | Status: RESOLVED

LayerZeroDVNSlasher implements slashing by allowing anyone to act as a challenger and submit a slashing request, deposit a bond, and allow the security council to cancel or fulfill the request. The token contract (bondToken) is set by the deployer upon deployment.

The current implementation contains both calls to transfer and transferFrom, and also uses the SafeERC20 to wrap around bondToken to perform a safeTransfer. The latter ensures reverting upon a failed transfer.

The current implementation is problematic if bondToken returns false when transfer (and transferFrom) fails rather than reverting (see here for examples of such tokens). As is, there is no check for success of calls to transfer and transferFrom functions, resulting in, for example, the possibility of a malicious challenger to submit a slashing request without paying a bond in case the transfer failed. Moreover, if the challenger creates such a slashing request, the securityCouncil may inadvertently transfer the unpaid bond amount to the malicious challenger, if the request is fulfilled or canceled with refundBond set to true. This would essentially lead to the malicious challenger stealing the bond amount, and not leaving enough tokens for other challengers’ refunds.

CENTRALIZATION ISSUES

It is often desirable for DeFi protocols to assume no trust in a central authority, including the protocol’s owner. Even if the owner is reputable, users are more likely to engage with a protocol that guarantees no catastrophic failure even in the case the owner gets hacked/compromised. We list issues of this kind below. (These issues should be considered in the context of usage/deployment, as they are not uncommon. Several high-profile, high-value protocols have significant centralization threats.)

Trust assumptions for LayerZeroServiceManager

Centralization | Status: INFO

Comment: The admin role is removed from the system in this commit.

The contract LayerZeroServiceManager involves several privileged roles. Namely, the owner of the contract, an admin that can be set by the owner, the reward initiator, and the reward claimer. Each of these roles has privileges that raise centralization concerns, listed below:

Owner:

Add a pending admin
Remove a pending admin
Remove an admin
Set and remove appointee to grant permission for a specific function of the EigenLayer core contracts
Set the address authorized to initiate rewards
Set the address authorized to claim rewards
Update the metadata URI for the AVS

Admin:

Add a pending admin
Remove a pending admin
Remove an admin
Set and remove appointee to grant permission for a specific function of the EigenLayer core contracts
Slash operators (see H1).
Change the AVS Registrar
Update the metadata URI for the AVS
Create new operator sets
Add and remove strategies from operator sets
Deregister operators (currently not possible because LayerZeroRegistryCoordinator does not allow deregistration but can be enabled by changing the AVS Registrar)
Change the address authorized to claim rewards

Reward Initiator:

Create new reward submissions to the EigenLayer RewardsCoordinator contract.

Reward Claimer:

Claim rewards from the EigenLayer RewardsCoordinator contract and transfer them to a chosen recipient.

Trust assumptions for LayerZeroDVNSlasher

Centralization | Status: INFO

The contract LayerZeroDVNSlasher involves a privileged role, namely the security council, which is authorized to cancel or fulfil slash requests. The council is also authorized to choose whether to refund or not the challenger bond upon cancellation of a slashing request. While this role is likely to be given to an account controlled by multiple physical entities using multisignatures, a centralization issue is still raised as whoever (one or more people) control this account can choose who can be slashed.

OTHER / ADVISORY ISSUES

This section details issues that are not thought to directly affect the functionality of the project, but we recommend considering them.

TimeboundSlasher::queueSlashingRequest() can be called directly

Advisory | Status: RESOLVED

The TimeboundSlasher contract serves as an intermediate layer where all slashing operations originate from the LayerZeroDVNSlasher contract. This is enforced by adding the onlySlasher modifier to functions TimeboundSlasher::cancelSlashingRequest() and TimeboundSlasher::fulfillSlashingRequest(). On the contrary, TimeboundSlasher::queueSlashingRequest() function has no access control. While this does not have direct security implications, it is recommended to restrict all slashing interactions through the LayerZeroDVNSlasher contract by adding onlySlasher modifier also to TimeboundSlasher::queueSlashingRequest().

Notes on reentrancy for LayerZeroDVNSlasher functions

Advisory | Status: INFO

The functions LayerZeroDVNSlasher::cancelSlashingRequest() and LayerZeroDVNSlasher::fulfillSlashingRequest() perform transfers of the bond token to the challenger. Currently the bond token is a trusted ERC20 token chosen when the contract is deployed. However, it’s worth noting that if in future changes the developers decided to use native ETH for bonding, then reentrancy can be a concern. Both functions do not follow the check-effects-interactions pattern and are not guarded against reentrancy. That’s been said, these functions are only callable by the security council so the only function that can be reentered is LayerZeroDVNSlasher::queueSlashingRequest() which does not lead to any security issues. However, since future code changes may incur logic changes or introduce new functions, it’s still important to be aware of this security consideration.

Compiler bugs

Advisory | Status: INFO

The code is compiled with Solidity 0.8.27. Version 0.8.27, in particular, has no known bugs.

DISCLAIMER

The audited contracts have been analyzed using automated techniques and extensive human inspection in accordance with state-of-the-art practices as of the date of this report. The audit makes no statements or warranties on the security of the code. On its own, it cannot be considered a sufficient assessment of the correctness of the contract. While we have conducted an analysis to the best of our ability, it is our recommendation for high-value contracts to commission several independent audits, a public bug bounty program, as well as continuous security auditing and monitoring through Dedaub Security Suite.

ABOUT DEDAUB

Dedaub offers significant security expertise combined with cutting-edge program analysis technology to secure some of the most prominent protocols in DeFi. The founders, as well as many of Dedaub’s auditors, have a strong academic research background together with a real-world hacker mentality to secure code. Protocol blockchain developers hire us for our foundational analysis tools and deep expertise in program analysis, reverse engineering, DeFi exploits, cryptography and financial mathematics.

APPENDIX

Proof of Concept - H1

The PoC uses the test setup provided in ServiceManagerIntegrationTest contract in the file LayerZeroDVNSlasherE2E.t.sol.

function testAdminSlash() public {
   vm.startPrank(address(timelockController));

   address admin = makeAddr("admin");
   serviceManager.addPendingAdmin(admin);

   vm.startPrank(admin);

   permissionController.acceptAdmin(address(serviceManager));

   uint256[] memory wads = new uint256[](strategies.length);
   for (uint i = 0; i < strategies.length; i++) {
       wads[i] = wadsToSlash;
   }

   IAllocationManagerTypes.SlashingParams memory params = IAllocationManagerTypes.SlashingParams({
       operator: registryCoordinator.operator(),
       operatorSetId: 0,
       strategies: strategies,
       wadsToSlash: wads,
       description: "test"
   });

   allocationManager.slashOperator(address(serviceManager), params);
   vm.stopPrank();

   // Assert operator has been slashed by checking their magnitude is 0 in each strategy
   for (uint256 i = 0; i < strategies.length; i++) {
       uint256 operatorMagnitude = allocationManager.getAllocation(
           operator, OperatorSet({avs: address(serviceManager), id: 0}), strategies[i]
       ).currentMagnitude;
       assertEq(operatorMagnitude, 0, "Operator magnitude should be 0 after slashing");
   }
}

SUMMARY

ABSTRACT

BACKGROUND

SETTING & CAVEATS

VULNERABILITIES & FUNCTIONAL ISSUES

CRITICAL SEVERITY

HIGH SEVERITY

Admin of the AVS can perform immediate slashes

MEDIUM SEVERITY

LayerZeroServiceManager cannot be used after setting an admin

LOW SEVERITY

Unsafe calls to external ERC20 bond token contract

CENTRALIZATION ISSUES

Trust assumptions for LayerZeroServiceManager

Trust assumptions for LayerZeroDVNSlasher

OTHER / ADVISORY ISSUES

TimeboundSlasher::queueSlashingRequest() can be called directly

Notes on reentrancy for LayerZeroDVNSlasher functions

Compiler bugs

DISCLAIMER

ABOUT DEDAUB

APPENDIX

Proof of Concept - H1