code-423n4 / 2024-01-curves-findings

1 stars 0 forks source link

Curves contains no way to recover stuck Ether` #1472

Closed c4-bot-2 closed 10 months ago

c4-bot-2 commented 10 months ago

Lines of code

https://github.com/code-423n4/2024-01-curves/blob/516aedb7b9a8d341d0d2666c23780d2bd8a9a600/contracts/Curves.sol#L246-L250

Vulnerability details

Impact

Ether can get stuck in Curves by buying tokens and sending excess Ether or the FeeSplitter being set to the zero address. In these situations funds are stuck inside the contract and theres no way to recover them.

Proof of Concept

https://github.com/code-423n4/2024-01-curves/blob/516aedb7b9a8d341d0d2666c23780d2bd8a9a600/contracts/Curves.sol#L246-L250

if (feesEconomics.holdersFeePercent > 0 && address(feeRedistributor) != address(0)) {
                feeRedistributor.onBalanceChange(curvesTokenSubject, msg.sender);
                feeRedistributor.addFees{value: holderFee}(curvesTokenSubject);
            }

Tools Used

Manual Review

Recommended Mitigation Steps

Either return excess Ether to the user and return excess funds if no FeeSplitter is set or add a rescue mechanism for trusted accounts.

Assessed type

ETH-Transfer

c4-pre-sort commented 10 months ago

raymondfam marked the issue as sufficient quality report

c4-pre-sort commented 10 months ago

raymondfam marked the issue as duplicate of #918

c4-pre-sort commented 10 months ago

raymondfam marked the issue as duplicate of #403

c4-judge commented 9 months ago

alcueca changed the severity to 2 (Med Risk)

c4-judge commented 9 months ago

alcueca marked the issue as satisfactory