code-423n4 / 2023-04-eigenlayer-findings

1 stars 1 forks source link

Medium Access Control Unauthorized access to restricted functions #376

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-04-eigenlayer/blob/5e4872358cd2bda1936c29f460ece2308af4def6/src/contracts/core/StrategyManager.sol#L839 https://github.com/code-423n4/2023-04-eigenlayer/blob/5e4872358cd2bda1936c29f460ece2308af4def6/src/contracts/core/StrategyManager.sol#L846 https://github.com/code-423n4/2023-04-eigenlayer/blob/5e4872358cd2bda1936c29f460ece2308af4def6/src/contracts/core/StrategyManager.sol#L592 https://github.com/code-423n4/2023-04-eigenlayer/blob/5e4872358cd2bda1936c29f460ece2308af4def6/src/contracts/core/StrategyManager.sol#L607

Vulnerability details

Impact

compromising the owner or whitelister roles could allow an attacker to manipulate critical whitelist and configuration logic, potentially damaging the use of the protocol. By taking control of privileged access, the attacker could enable rapid withdrawals, manipulate strategy inclusion, or make any other unwanted changes that disrupt the intended functionality.

Proof of Concept

Here is a possible proof of concept for unauthorized access to restricted functions:

address attacker = 0xabc123...; // address of malicious actor address owner = 0xdef456...; // address of the contract owner

// compromise the owner's account somehow owner = attacker;

// now the attacker controls the onlyOwner modifier

strategyWhitelister = 0xghij789...; // set the whitelister to the attacker's address // caller is now the attacker, able to access whitelist management functions

addStrategiesToDepositWhitelist([strategy1, strategy2, ...]); // add any strategies desired removeStrategiesFromDepositWhitelist([strategy3, strategy4, ...]); // remove any strategies

setWithdrawalDelayBlocks(0); // set delay to 0, enabling fast withdrawals

// the attacker now controls the whitelist and configuration, // able to manipulate the contract as they wish through the compromised access.

This proof of concept demonstrates how compromising either the owner role or strategy whitelister address could enable unauthorized access to the restricted management functions. By taking control of one of these privileged roles, an attacker could manipulate the whitelist, enable rapid withdrawals by lowering delays, or make any other unwanted changes to the configuration and logic.

Tools Used

The risk was identified through manual auditing and penetration testing of the smart contract code. A basic proof of concept was constructed to illustrate the potential damage of this vulnerability

Recommended Mitigation Steps

To secure privileged roles and management functions, the following steps should be applied:

Only grant privileged access (owner, whitelister) to trusted parties. Conduct a review of any accounts/addresses with special permissions.

Apply multi-signature requirements or timelocks to privileged roles whenever possible. This limits the impact of any single account compromise.

Consider using separate roles for management vs. deployment/emergency use. Management roles should only manipulate configuration/whitelist, deployment roles should handle rare functionality changes.

Regularly audit the use of privileged functions to ensure they remain limited to intended and legitimate use cases. Look for any suspicious activity that could indicate compromise.

Apply additional access control around sensitive management functions with modifiers, gates, etc. Whenever possible, limit access to necessities only.

Consider an on-chain storage and voting mechanism for whitelisted strategies. This decentralizes control and validation, reducing the impact of a single whitelister compromise.

Require the use of "refund / revert config change" functions for any sensitive management actions. These could be used to fix issues if compromise is detected, containing damage.

Conduct risk management reviews of all privileges and how they could potentially be misused. Identify any new sensitive roles/permissions before implementation.

Monitor security best practices for new recommended controls. The field is constantly evolving, incremental improvements should be applied regularly.

Assessed type

Access Control

0xSorryNotSorry commented 1 year ago

Spam.

c4-pre-sort commented 1 year ago

0xSorryNotSorry marked the issue as low quality report

c4-judge commented 1 year ago

GalloDaSballo marked the issue as unsatisfactory: Invalid