aman - `RETHAdapter:prefundedRedeem` allows user to withdraw donated ETH

[sherlock-admin]

aman

medium

RETHAdapter:prefundedRedeem allows user to withdraw donated ETH

Summary

RETHAdapter:prefundedRedeem allow the to address to Redeem RETH and than transfer the total WETH balance of RETHAdapter contract to to address. However there might be donated ETH in this Contract.

Vulnerability Detail

The RETHAdapter:prefundedRedeem(to) function perform the follwoing process. 1). Extract the RETH balance of this contract. 2). Burn the RETH token of this contract and receive the ETH from RocketPool. 3). Get the ETH balance of this contract and convert it into WETH. 4). transfer the WETH to to address. There might be some donated ETH in this contract for which recoverETH function is implemented to recover the Donated ETH by Admin. The Issue in the current implmentaton is RETHAdapter:prefundedRedeem(to) does not care about the donated ETH and convert address(this).balance to WETH.

LOC

https://github.com/sherlock-audit/2024-01-napier/blob/6313f34110b0d12677b389f0ecb3197038211e12/napier-v1/src/adapters/rocketPool/RETHAdapter.sol#L82C3-L98C1

Impact

The user Will receive more WETH than expected.

Code Snippet

  function prefundedRedeem(address to) external override returns (uint256, uint256) {
        // Load contracts
        uint256 rethBal = IRocketETH(RETH).balanceOf(address(this));
        if (rethBal == 0) {
            return (0, 0);
        }
        // Burn rETH for ETH
        IRocketETH(RETH).burn(rethBal);
        // Wrap ETH to WETH
@>      uint256 ethValue = address(this).balance;
@>        IWETH9(WETH).deposit{value: ethValue}();
        // Transfer WETH to recipient
@>        IWETH9(WETH).safeTransfer(to, ethValue);

        return (ethValue, rethBal);
    }

POC

    function testPrefundedRedeemWithDonatedETH() public  {
        deal(address(adapter), 1 ether);

        // 0) transfer rETH to the adapter contract prior as it would be done by Tranche
        uint256 rEthFundedAmount = rEthBalanceBefore;
        _fundAdapterTarget(rEthFundedAmount);

        // 1) call prefundedRedeem to unwrap rETH to WETH, and check expected return amount
        (uint256 amountWithdrawn, ) = adapter.prefundedRedeem(user);
        uint256 expectedWEthRedeemed = IRocketTokenRETH(RETH).getEthValue(rEthFundedAmount);

        assertGt(
            amountWithdrawn,
            expectedWEthRedeemed,
            "actual WETH redeemed !~= expected WETH redeemed on testPrefundedRedeem()"
        );
    }

Add this test case inside RETHAdapter.t.sol and run with command forge test --mt testLLPrefundedRedeem -vvv.

Tool used

Manual Review

Recommendation

Cache the Ether Balance of Contract before calling IRocketETH(RETH).burn(rethBal); and minus it from Ether balance after. then converted the subtracted value into WETH and trasfer it to to address.

diff --git a/napier-v1/src/adapters/rocketPool/RETHAdapter.sol b/napier-v1/src/adapters/rocketPool/RETHAdapter.sol
index 50a8a73..75feb9d 100644
--- a/napier-v1/src/adapters/rocketPool/RETHAdapter.sol
+++ b/napier-v1/src/adapters/rocketPool/RETHAdapter.sol
@@ -85,10 +85,13 @@ contract RETHAdapter is BaseAdapter {
         if (rethBal == 0) {
             return (0, 0);
         }
+        // @audit : should check for diff before and after
         // Burn rETH for ETH
+        uint256 ethBefore = address(this).balance;
+
         IRocketETH(RETH).burn(rethBal);
         // Wrap ETH to WETH
-        uint256 ethValue = address(this).balance;
+        uint256 ethValue = address(this).balance - ethBefore;
         IWETH9(WETH).deposit{value: ethValue}();
         // Transfer WETH to recipient
         IWETH9(WETH).safeTransfer(to, ethValue);

[sherlock-admin]

2 comment(s) were left on this issue during the judging contest.

tsvetanovv commented:

Low. I don't think this is a medium vulnerability

takarez commented:

valid: the function does infact convert including the donated ETH ; medium(12)