code-423n4 / 2023-10-ethena-findings

5 stars 5 forks source link

EthenaMinting.Order should be ordered by `nonce`. #731

Open c4-submissions opened 11 months ago

c4-submissions commented 11 months ago

Lines of code

https://github.com/code-423n4/2023-10-ethena/blob/main/contracts/EthenaMinting.sol#L377-L386

Vulnerability details

Impact

A user's pending tx to EthenaMinting.mint or redeem can be run after a long time even if the invalidatorSlot of the tx is lower than the invalidatorSlot of the previous tx.

Proof of Concept

Add the below test in EthenaMinting.core.t.sol and run forge test --mt testPOC_nativeEth_withdraw_twotimes. The test changes invalidatorSlot 3 -> 0 -> 1. The invalidatorSlot, however, should not decrease.

  function testPOC_nativeEth_withdraw_twotimes() public {
    vm.deal(address(EthenaMintingContract), _stETHToDeposit);

    // @audit invalidaotSlot: 3
    IEthenaMinting.Order memory order = IEthenaMinting.Order({
      order_type: IEthenaMinting.OrderType.MINT,
      expiry: block.timestamp + 10 minutes,
      nonce: 800,
      benefactor: benefactor,
      beneficiary: benefactor,
      collateral_asset: address(stETHToken),
      collateral_amount: _stETHToDeposit,
      usde_amount: _usdeToMint
    });

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

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

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

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

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

    assertEq(usdeToken.balanceOf(benefactor), 0);

    vm.recordLogs();
    vm.prank(minter);
    EthenaMintingContract.mint(order, route, takerSignature);
    vm.getRecordedLogs();

    assertEq(usdeToken.balanceOf(benefactor), _usdeToMint);

    // redeem1
    // @audit invalidaotSlot: 0
    IEthenaMinting.Order memory redeemOrder = IEthenaMinting.Order({
      order_type: IEthenaMinting.OrderType.REDEEM,
      expiry: block.timestamp + 10 minutes,
      nonce: 8,
      benefactor: benefactor,
      beneficiary: benefactor,
      collateral_asset: NATIVE_TOKEN,
      usde_amount: _usdeToMint / 2,
      collateral_amount: _stETHToDeposit / 2
    });

    // taker
    vm.startPrank(benefactor);
    usdeToken.approve(address(EthenaMintingContract), _usdeToMint);

    bytes32 digest2 = EthenaMintingContract.hashOrder(redeemOrder);
    IEthenaMinting.Signature memory takerSignature2 =
      signOrder(benefactorPrivateKey, digest2, IEthenaMinting.SignatureType.EIP712);
    vm.stopPrank();

    vm.startPrank(redeemer);
    EthenaMintingContract.redeem(redeemOrder, takerSignature2);

    // redeem2
    // @audit invalidaotSlot: 1
    IEthenaMinting.Order memory redeemOrder2 = IEthenaMinting.Order({
      order_type: IEthenaMinting.OrderType.REDEEM,
      expiry: block.timestamp + 10 minutes,
      nonce: 257,
      benefactor: benefactor,
      beneficiary: benefactor,
      collateral_asset: NATIVE_TOKEN,
      usde_amount: _usdeToMint / 2,
      collateral_amount: _stETHToDeposit / 2
    });
    vm.stopPrank();

    // taker
    vm.startPrank(benefactor);
    usdeToken.approve(address(EthenaMintingContract), _usdeToMint);

    bytes32 digest3 = EthenaMintingContract.hashOrder(redeemOrder2);
    IEthenaMinting.Signature memory takerSignature3 =
            signOrder(benefactorPrivateKey, digest3, IEthenaMinting.SignatureType.EIP712);
    vm.stopPrank();

    vm.startPrank(redeemer);
    EthenaMintingContract.redeem(redeemOrder2, takerSignature3);

    assertEq(stETHToken.balanceOf(benefactor), 0);
    assertEq(usdeToken.balanceOf(benefactor), 0);
    assertEq(benefactor.balance, _stETHToDeposit);

    vm.stopPrank();
  }

Tools Used

Manual, foundry

Recommended Mitigation Steps

Mange lastInvalidatorSlot in EthenaMinting to order transactions.

contract EthenaMinting is IEthenaMinting, SingleAdminAccessControl, ReentrancyGuard {
  // @audit Store the last invalidatorSlot. 
  uint256 lastInvalidatorSlot;

  function verifyNonce(address sender, uint256 nonce) public view override returns (bool, uint256, uint256, uint256) {
    if (nonce == 0) revert InvalidNonce();
    uint256 invalidatorSlot = uint64(nonce) >> 8;
    uint256 invalidatorBit = 1 << uint8(nonce);

    // @audit Add this one line.   
    if (invalidatorSlot < lastInvalidatorSlot) revert InvalidNonce();

    mapping(uint256 => uint256) storage invalidatorStorage = _orderBitmaps[sender];
    uint256 invalidator = invalidatorStorage[invalidatorSlot];
    if (invalidator & invalidatorBit != 0) revert InvalidNonce();

    return (true, invalidatorSlot, invalidator, invalidatorBit);
  }

  function _deduplicateOrder(address sender, uint256 nonce) private returns (bool) {
      (bool valid, uint256 invalidatorSlot, uint256 invalidator, uint256 invalidatorBit) = verifyNonce(sender, nonce);
      mapping(uint256 => uint256) storage invalidatorStorage = _orderBitmaps[sender];
      // @audit Add this one line.
      if (lastInvalidatorSlot < invalidatorSlot) lastInvalidatorSlot = invalidatorSlot;
      invalidatorStorage[invalidatorSlot] = invalidator | invalidatorBit; // update invalidatorStorage
      return valid;
  }

Assessed type

Other

c4-pre-sort commented 11 months ago

raymondfam marked the issue as sufficient quality report

c4-pre-sort commented 11 months ago

raymondfam marked the issue as primary issue

FJ-Riveros commented 11 months ago

This shouldn't present any risk at all.

c4-sponsor commented 11 months ago

FJ-Riveros (sponsor) disputed

c4-judge commented 11 months ago

fatherGoose1 changed the severity to QA (Quality Assurance)

fatherGoose1 commented 11 months ago

Agree that this does not impose any risk. QA as it is a useful design recommendation.

c4-judge commented 11 months ago

fatherGoose1 marked the issue as grade-b