Unused imports and Upgradeability problem of EigenPod.

[code423n4]

Lines of code

https://github.com/code-423n4/2023-04-eigenlayer/blob/main/src/contracts/pods/EigenPod.sol#L2-L34

Vulnerability details

Impact

First the EigenPod.sol file imports OwnableUpgradeable and AddressUpgradeable but doesn't use them. The file only uses ReentrancyUpgradeable but it only helps protect the contract from re-entrancy vulnerability and it does not provide any upgradeability functionality by itself. Therefore, it seems that the EigenPod contract is not upgradeable. If there are any issues or updates needed in the future, the contract would not be upgradeable, and users would have to migrate to a new contract. Specifically, the EigenPodManager.sol file is closely related to EigenPod.sol but EigenPodManager seems correct and upgradeable.

Proof of Concept

https://github.com/code-423n4/2023-04-eigenlayer/blob/main/src/contracts/pods/EigenPod.sol#L2-L34 `` pragma solidity =0.8.12;

import "@openzeppelin-upgrades/contracts/proxy/utils/Initializable.sol"; import "@openzeppelin-upgrades/contracts/access/OwnableUpgradeable.sol"; import "@openzeppelin-upgrades/contracts/security/ReentrancyGuardUpgradeable.sol"; import "@openzeppelin-upgrades/contracts/utils/AddressUpgradeable.sol";

import "../libraries/BeaconChainProofs.sol"; import "../libraries/BytesLib.sol"; import "../libraries/Endian.sol";

import "../interfaces/IETHPOSDeposit.sol"; import "../interfaces/IEigenPodManager.sol"; import "../interfaces/IEigenPod.sol"; import "../interfaces/IDelayedWithdrawalRouter.sol"; import "../interfaces/IPausable.sol";

import "./EigenPodPausingConstants.sol";

/**

• @title The implementation contract used for restaking beacon chain ETH on EigenLayer
• @author Layr Labs, Inc.
• @notice The main functionalities are:
• • creating new ETH validators with their withdrawal credentials pointed to this contract
• • proving from beacon chain state roots that withdrawal credentials are pointed to this contract
• • proving from beacon chain state roots the balances of ETH validators with their withdrawal credentials
• pointed to this contract
• • updating aggregate balances in the EigenPodManager
• • withdrawing eth when withdrawals are initiated
• @dev Note that all beacon chain balances are stored as gwei within the beacon chain datastructures. We choose
• to account balances in terms of gwei in the EigenPod contract and convert to wei when making calls to other contracts */ contract EigenPod is IEigenPod, Initializable, ReentrancyGuardUpgradeable, EigenPodPausingConstants { ...... ``

Tools Used

Foundry

Recommended Mitigation Steps

If you're looking to make your contract upgradeable, you will need to use additional components provided by the OpenZeppelin Contracts Upgrades library, like Upgradeable, UUPSUpgradeable, or other upgrade patterns.

For instance, if you want to use the UUPS (Universal Upgradeable Proxy Standard) pattern, your contract should inherit from UUPSUpgradeable:

import "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol";

contract EigenPod is IEigenPod, Initializable, ReentrancyGuardUpgradeable, EigenPodPausingConstants, UUPSUpgradeable {
    // Contract code here
}

Remember that you also need to implement the _authorizeUpgrade function, which is required by the UUPSUpgradeable contract:

function _authorizeUpgrade(address) internal override {
    // Custom access control code here, e.g., using OwnableUpgradeable or any custom solution
}

Additionally, you'll need to deploy your contract using a Proxy, like TransparentUpgradeableProxy or UUPSUpgradeableProxy.

Also since you import OwnableUpgradeable and AddressUpgradeable files, please double-check whether they are needed in the EigenPod contract.

Assessed type

Upgradable

[0xSorryNotSorry]

UUPSUpgradeable is a way of using proxy mechanism to have the upgradable functionality of a contract but it's not the sole necessity. The devs opted to the have storage gaps (uint256[46] private __gap;) in EigenPod contract as this contract is served like a main template for the Pod deployments. Thinking of there will be a number of pod owners, it will not be ideal to update all of them but update the existing pod contract for future deployments. Invalid assumption.

[c4-pre-sort]

0xSorryNotSorry marked the issue as low quality report

[GalloDaSballo]

Invalid because the Sponsor uses BeaconProxy

[c4-judge]

GalloDaSballo marked the issue as unsatisfactory: Invalid