code-423n4 / 2023-11-kelp-findings

13 stars 11 forks source link

Missing `whenNotPaused` modifier in some state-changing functions #417

Closed c4-submissions closed 11 months ago

c4-submissions commented 11 months ago

Lines of code

https://github.com/code-423n4/2023-11-kelp/blob/f751d7594051c0766c7ecd1e68daeb0661e43ee3/src/LRTOracle.sol#L88 https://github.com/code-423n4/2023-11-kelp/blob/f751d7594051c0766c7ecd1e68daeb0661e43ee3/src/NodeDelegator.sol#L38 https://github.com/code-423n4/2023-11-kelp/blob/f751d7594051c0766c7ecd1e68daeb0661e43ee3/src/LRTDepositPool.sol#L183 https://github.com/code-423n4/2023-11-kelp/blob/f751d7594051c0766c7ecd1e68daeb0661e43ee3/src/LRTDepositPool.sol#L202 https://github.com/code-423n4/2023-11-kelp/blob/f751d7594051c0766c7ecd1e68daeb0661e43ee3/src/LRTDepositPool.sol#L162

Vulnerability details

Impact

LRTOracle, NodeDelegator, LRTDepositPool inherits from PausableUpgradeable which implies, that these contracts can be paused. However, some functions from these contracts can be called even when the contract is in a paused state. Paused state suggests that there shouldn't be any actions nor changes on the current, paused contract.

Proof of Concept

Function updatePriceOracleFor() does not implement whenNotPaused modifier. It's possible to add/update the price oracle of any supported asset even when the contract is in the paused state.

Function maxApproveToEigenStrategyManager() does not implement whenNotPaused modifier. It allows to approve the maximum amount of asset to the Eigen strategy manager, even, when the contract is in the paused state.

Function addNodeDelegatorContractToQueue() does not implement whenNotPaused modifier. It's possible to add new node delegator contract addresses, even when the contract is in the paused state.

Function transferAssetToNodeDelegator() does not implement whenNotPaused modifier. It's possible to transfer asset lying in DepositPool to NodeDelegator contract, even, when the contract is in the paused state.

Function updateMaxNodeDelegatorCount() does not implement whenNotPaused modifier. It allows to update the max node delegator count, even, when the contract is in the paused state.

Tools Used

Manual code review

Recommended Mitigation Steps

Implement whenNotPaused modifier for mentioned functions

Assessed type

Access Control

c4-pre-sort commented 11 months ago

raymondfam marked the issue as insufficient quality report

c4-pre-sort commented 11 months ago

raymondfam marked the issue as duplicate of #318

c4-judge commented 10 months ago

fatherGoose1 marked the issue as unsatisfactory: Invalid