Othentic

The original version of the protocol delegated share syncing - the process of updating how much voting power an operator had based on their underlying balance - to a centralised role. This left the protocol open to an attack where the share syncer could maliciously update the balances to pass or reject any task they wanted.

The new version of the protocol introduces the concept of system tasks. System tasks are still performed by operators like any other tasks, however these are specifically designed to assist in the operation of the AVS (in this case updating the share balances). This solution has been generalised, with AVS’s able to implement their own tasks via the InternalTaskHandler.

function _votingPowerUpdate(IAttestationCenter.TaskInfo memory _task) internal {
    InternalTaskHandlerStorageData storage _sd = _getStorage();
    IOBLS _obls = _sd.obls;
    VotingPowerUpdate memory _update = abi.decode(_task.data, (VotingPowerUpdate));
    uint _lastCommitBlockL1 = _sd.lastCommitBlockL1;
    uint _lastCommitBlockL2 = _sd.lastCommitBlockL2;
    if (_lastCommitBlockL1 >= _update.toBlockL1) {
        revert InvalidToBlockL1VsLastCommitBlockL1(_lastCommitBlockL1+1);
    }
    if (_lastCommitBlockL2 >= _update.toBlockL2) {
        revert InvalidToBlockL2VsLastCommitBlockL2(_lastCommitBlockL2+1);
    }
    if (_update.toBlockL2 >= block.number) {
        revert InvalidToBlockL2VsCurrentHeight(_update.toBlockL2, block.number);
    }
    _sd.lastCommitBlockL1 = _update.toBlockL1;
    _sd.lastCommitBlockL2 = _update.toBlockL2;
    if (_update.toIncrease.length > 0) {
        _obls.increaseBatchOperatorVotingPower(_update.toIncrease);
    }
    if (_update.toDecrease.length > 0) {
        _obls.decreaseBatchOperatorVotingPower(_update.toDecrease);
    }
    emit VotingPowerUpdated(_update.toBlockL1, _update.toBlockL2, _task.proofOfTask);
}

The original version of the protocol included special handling for taskDefinitionId zero. The rationale behind this was to have a “default” task to speed up developer onboarding. We disliked this approach as special casing, especially for validation, generally leads to issues as the code evolves and the invariants are forgotten.

The Othentic team have now fixed these issues by implementing a default task which is created upon initialisation, and removed the special casing for taskDefinitionId zero.

In any AVS, an operator’s voting power is calculated in terms of the amount of shares which have been delegated to them from a set of supported strategies.

function votingPower(address _operator) external view returns (uint256) {
...
	return _getVotingPower(_sd, _operator, _votingPowerMultipliers);
}

function _getVotingPower(AvsGovernanceStorageData storage _sd, address _operator, IAvsGovernance.VotingPowerMultiplier[] memory _votingPowerMultipliers) private view returns (uint256) {
	...
	uint256 _votingPower = _sd.othenticRegistry.getVotingPower(_operator, _votingPowerMultipliers, address(this));
	...
	return _votingPower;

Inside the OthenticRegistry contract, each strategy is checked by either EigenLayer’s strategy manager or Symbiotic’s vault factory to determine whether it’s a valid strategy. If a strategy is deemed unsupported, the operator is considered to have 0 voting power based on that strategy.

function _getVotingPower(address _operator, IAvsGovernance.VotingPowerMultiplier calldata _votingPowerMultiplier, address _avsGovernance) private view returns (uint256) {
  if(!_isValidStakingContract(_votingPowerMultiplier.stakingContract)) {
	return 0;
  }
...

function _isValidStakingContract(address _stakingContract) private view returns (bool) {
  return strategyManager.strategyIsWhitelistedForDeposit(IStrategy(_stakingContract)) || vaultFactory.isEntity(_stakingContract);
}

One of the EigenLayer strategies that is supported by default in newly setup AVSs is the Beacon Chain ETH strategy

address private constant BEACON_CHAIN_ETH_STRATEGY = 0xbeaC0eeEeeeeEEeEeEEEEeeEEeEeeeEeeEEBEaC0;
...
function getDefaultStrategies(uint _chainid) external pure returns (IAvsGovernance.StakingContractInfo[] memory) {
	if (_chainid == 1) {
    	IAvsGovernance.StakingContractInfo[] memory _strategies = new IAvsGovernance.StakingContractInfo[](14);
    	...
    	_strategies[12] = IAvsGovernance.StakingContractInfo(BEACON_CHAIN_ETH_STRATEGY, IAvsGovernance.SharedSecurityProvider.EigenLayer); // Beacon Chain ETH strategy
    	...

The problem lies in the fact the strategy manager (ETH:0x858646372CC42E1A627fcE94aa7A7033e7CF075A) does not consider the Beacon Chain ETH strategy as whitelisted for deposit, meaning that strategyIsWhitelistedForDeposit will return false when attempting to calculate an operator’s voting power based on that strategy. The core EigenLayer contracts handle this address as a sentinel value, and the entity that records deposits and withdrawals on it is the EigenPodManager, which is what the DelegationManager contract consults in order to determine an operator’s shares on the Beacon Chain ETH strategy.

As a consequence, OthenticRegistry::getVotingPower will return 0 for this strategy and so the protocol’s accounting will not consider any voting power coming out of the Beacon Chain ETH.

When an AvsGovernance contract is initialized, there are some state parameters that are not initialized directly in AvsGovernance::initialize:

...
AvsGovernanceStorageData storage _sd = _getStorage();
_sd.othenticRegistry = _othenticRegistry;
_sd.messageHandler = IMessageHandler(_messageHandler);
_grantRole(RolesLibrary.MESSAGE_HANDLER, _messageHandler);
_sd.avsTreasury = IAvsTreasury(_initializationParams.avsTreasury);
_sd.allowlistSigner = _initializationParams.allowlistSigner;
_sd.rewardsReceiverModificationDelay = 7 days;
_sd.avsDirectoryContract = IAVSDirectory(_initializationParams.avsDirectoryContract);
_sd.numOfOperatorsLimit = 100;
_sd.blsAuthSingleton = _initializationParams.blsAuthSingleton;
_setAvsName(_sd, _avsName);
...

For the minStakeAmountPerStakingContract mapping as well as the minVotingPower and isAllowlisted state variables, an appropriate setter function must be used.

Because the corresponding setter functions can only be called by the AVS Goverance Multisig, and those parameters cannot be set atomically during initialization, there may arise some race conditions after the contract’s initialization:

• With minVotingPower and minStakeAmountPerStakingContract being unset, an operator may register without having any shares. This could lead to scenarios where someone bundles multiple transactions ( using different contracts or tx.origins ) right after the contract’s initialization to register as an operator. This could lead to a temporary DOS where the numOfOperatorsLimit is hit and legitimate operators are temporarily unable to register (until the limit is increased)

...
bool _isActive = (_votingPower >= _sd.minVotingPower) && _sd.othenticRegistry.isValidStakeAmount(_operator, _minStakePerStakingContract, address(this));
if (!_isActive) revert NotEnoughVotingPower();
...

• Both with and without the above-mentioned limits being set, someone can always front-run calls to AvsGovernance::setIsAllowlisted and register before the whitelisting functionality is turned on

function registerAsOperator(OperatorRegistrationParams calldata _operatorRegistrationParams) ... {
	AvsGovernanceStorageData storage _sd = _getStorage();
	if (_sd.isAllowlisted) {
    		if (_operatorRegistrationParams.authToken.length == 0) {
        		revert MissingAuthToken(_operatorRegistrationParams.authToken);
    		}
    		...
        }
...

It should be noted that transactions using the setters are susceptible to front-running unless bundled with a service like Flashbots, even if sent shortly after the contract’s initialization.

One way to mitigate these considerations altogether is to allow the corresponding storage fields to be set atomically during the contract’s initialization.

When registering as an operator in the AVSGovernance via registerAsOperator, operators are first expected to call registerOperatorToEigenLayer and/or registerOperatorToSymbiotic.

If an operator fails to call registerOperatorToEigenLayer prior to registerAsOperator its votes will still be calculated correctly, as it calculates the share amount from the DelegationManager (via the OthenticRegistry) which does a direct storage lookup. However if an operator fails to call registerOperatorToSymbiotic prior to registerAsOperator it will have zero voting power, as BaseDelegator.stakeAt consults the OptInService to determine if the operator has registered with the AVS and will return zero in the case it has not been registered.

Using an enum to represent SharedSecurityProvider in AvsGovernance can lead to compatibility issues with non-upgradeable contracts if the enum is ever expanded.

This is because the compiler automatically inserts range checks for enum values, regardless of how the enum value is used. As a result, adding new enum values later can cause existing non-upgradeable contracts to fail when they encounter values outside the original range.

Comments:
This finding does not directly impact the Othentic protocol as all the contracts interacting with this enum are upgradeable. We have chosen to leave this finding in the report as advice to any developers planning to integrate with Othentic, and we would also like to note that this is not specific to Othentic but to Solidity enums generally.

function _initialize(InitializationParams calldata _initializationParams) internal onlyInitializing {
...
	_othenticRegistry.registerAvs(_avsName);
...
}

function registerAvs(string memory _avsName) external {
  emit AvsOptIn(msg.sender, _avsName);
}

Although there are no security consequences on-chain nor off-chain, developers might consider disallowing the empty string from being a valid AVS name.

The following functions are not used internally anywhere inside the AvsGovernance contract and thus can be marked as external

function setMinStakesForStakingContract(address _stakingContract, uint256 _minShares) public ... {

function setStakingContractMultiplierBatch(VotingPowerMultiplier[] calldata _votingPowerMultipliers) public ... {

The following initialization is not necessary

function _initialize(InitializationParams calldata _initializationParams) internal onlyInitializing {
...
	_sd.numOfTotalOperators = 0;
...

The storage gap in the OthenticRegistry contract is 45 slots

   // slither-disable-next-line unused-state,naming-convention
  uint256[45] private __gap;

However, given the contract’s storage layout and the convention of allocating a total of 50 slots in gap storage, the developers could consider making the current gap be 41 slots.

contract OthenticRegistry is Initializable, OwnableUpgradeable, IOthenticRegistry {
...

  address private constant BEACON_CHAIN_ETH_STRATEGY = 0xbeaC0eeEeeeeEEeEeEEEEeeEEeEeeeEeeEEBEaC0;

  //Eigenlayer contracts
  ISlasher public slasher;
  IDelegationManager public delegationManager;
  IStrategyManager public strategyManager;

  //Othentic contracts
  AvsGovernances private avsGovernances; // obsolete

  //Symbiotic contracts
  IVaultFactory public vaultFactory;
  IOptInService public optInService;
  INetworkRegistry public networkRegistry;
  INetworkMiddlewareService public networkMiddlewareService;

  //Othentic contracts
  IL1AvsFactory public l1AvsFactory;

The code is compiled with Solidity version 0.8.25 which has no known bugs.