Ethena Labs - qpzm's results

Enabling The Internet Bond

General Information

Platform: Code4rena

Start Date: 24/10/2023

Pot Size: $36,500 USDC

Total HM: 4

Participants: 147

Period: 6 days

Judge: 0xDjango

Id: 299

League: ETH

Ethena Labs

Findings Distribution

Researcher Performance

Rank: 102/147

Findings: 1

Award: $4.52

QA:
grade-b

🌟 Selected for report: 0

🚀 Solo Findings: 0

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

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

#0 - c4-pre-sort

2023-11-01T03:41:28Z

raymondfam marked the issue as sufficient quality report

#1 - c4-pre-sort

2023-11-01T03:42:08Z

raymondfam marked the issue as primary issue

#2 - FJ-Riveros

2023-11-09T16:30:13Z

This shouldn't present any risk at all.

#3 - c4-sponsor

2023-11-09T16:30:19Z

FJ-Riveros (sponsor) disputed

#4 - c4-judge

2023-11-13T18:48:43Z

fatherGoose1 changed the severity to QA (Quality Assurance)

#5 - fatherGoose1

2023-11-13T18:49:16Z

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

#6 - c4-judge

2023-11-13T18:49:21Z

fatherGoose1 marked the issue as grade-b

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax © 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter