code-423n4 / 2024-07-karak-findings

0 stars 0 forks source link

The `Core` contract doesn't implement `IDSS.cancelUpdateStakeHook()`. #39

Open c4-bot-8 opened 2 months ago

c4-bot-8 commented 2 months ago

Lines of code

https://github.com/code-423n4/2024-07-karak/blob/main/src/Core.sol#L130 https://github.com/code-423n4/2024-07-karak/blob/main/src/Core.sol#L146 https://github.com/code-423n4/2024-07-karak/blob/main/src/interfaces/IDSS.sol#L14

Vulnerability details

Impact

There is no way to cancel a request to stake/unstake vaults to a DSS.

Proof of Concept

When an operator stakes/unstakes vaults to a DSS, the operator can request it using Core.requestUpdateVaultStakeInDSS(). After that, anyone can finish the request using Core.finalizeUpdateVaultStakeInDSS().

Core.requestUpdateVaultStakeInDSS() calls Operator.requestUpdateVaultStakeInDSS() and then IDSS.requestUpdateStakeHook(). Core.finalizeUpdateVaultStakeInDSS() calls Operator.validateAndUpdateVaultStakeInDSS() and then IDSS.finishUpdateStakeHook().

But there is additional functionality in IDSS. It's IDSS.cancelUpdateStakeHook(), and an operator can cancel his request using this method. But in the implementation of Core, there is no implementation to cancel the request to stake/unstake vaults to a DSS. As a result, request staking/unstaking have to be finialized without canceling.

Tools Used

Manual Review

Recommended Mitigation Steps

It is recommended to add implementation about canceling the request to stake/unstake vaults to a DSS in the Core contract.

Assessed type

Other

c4-judge commented 2 months ago

MiloTruck marked the issue as unsatisfactory: Out of scope

c4-judge commented 2 months ago

MiloTruck marked the issue as not a duplicate

c4-judge commented 2 months ago

MiloTruck removed the grade

c4-judge commented 2 months ago

MiloTruck changed the severity to QA (Quality Assurance)

MiloTruck commented 2 months ago

Will confirm with the sponsor, but if the Core contract doesn't call cancelUpdateStakeHook anywhere, it means that cancelling updates is not supported and the hook is not needed.

c4-judge commented 2 months ago

MiloTruck marked the issue as grade-b

devdks25 commented 2 months ago

Yeah cancelUpdateStake isn't implemented hence the hook's redundant.

devdks25 commented 1 month ago

Fix: https://github.com/karak-network/karak-restaking/pull/386