users will lose share if they call enterExitQueue multiple times.

Github username: @0xmahdirostami Submission hash (on-chain): 0xfed1c7f48c285532829557dcf3727f2cac7050bb38441eee146a4ac533afd6cb Severity: high

Description: Description\ As in the claimExitedAssets function the newPositionTicket is calculated in this way:

      newPositionTicket = positionTicket + claimedShares;
      _exitRequests[keccak256(abi.encode(msg.sender, newPositionTicket))] = leftShares;

AND

    uint256 requestedShares = _exitRequests[keccak256(abi.encode(receiver, positionTicket))];

If the receiver and positionTicket will be the same, Users could update their _exitRequests mapping accidentally.

Scenario\

State A:

_exitQueue.getLatestTotalTickets() = 10_000
queuedShares = 0

User A enterExitQueue witH 6_000 shares So

_exitQueue.getLatestTotalTickets() = 10_000
queuedShares = 6_000
User A positionTicket = 10_000
_exitRequests[keccak256(abi.encode(User A,positionTicket))] = 6_000

After Some time wants to get his share calls updatestate that will call _updateExitQueue and then calls claimExitedAssets: Maybe in that time, just 3_000(not a total of 6_000) shares are burned (As some validator might exit).

State B:
_exitQueue.getLatestTotalTickets() = 13_000
queuedShares = 3_000
User A New positionTicket = 13_000
_exitRequests[keccak256(abi.encode(User A, positionTicket))] = 3_000

User A enterExitQueue again with 1_000 share So

State C:
_exitQueue.getLatestTotalTickets() = 13_000
queuedShares = 4_000
User A positionTicket = 13_000
_exitRequests[keccak256(abi.encode(User A, positionTicket))] = 1_000

As you see User A's positionTicket changed.

Attachments

Revised Code File (Optional)

Use nonce for each user's enterExitQueue request. VaultState file

+  mapping(address => uint256) internal _nounces;

VaultEnterExit file

  function enterExitQueue(
    uint256 shares,
    address receiver
  ) public virtual override returns (uint256 positionTicket) {
    _checkCollateralized();
    if (shares == 0) revert Errors.InvalidShares();
    if (receiver == address(0)) revert Errors.ZeroAddress();

    // SLOAD to memory
    uint256 _queuedShares = queuedShares;
     positionTicket = _exitQueue.getLatestTotalTickets() + _queuedShares;

     // add to the exit requests
-    _exitRequests[keccak256(abi.encode(receiver, positionTicket))] = shares;
-
+    _exitRequests[keccak256(abi.encode(receiver, positionTicket, _nounces[receiver]))] = shares;
+    _nounces[receiver] += 1;

   function calculateExitedAssets(
     address receiver,
     uint256 positionTicket,
    uint256 exitQueueIndex
+    uint256 _nounce
   ) external view returns (uint256 leftShares, uint256 claimedShares, uint256 claimedAssets);
-    uint256 requestedShares = _exitRequests[keccak256(abi.encode(receiver, positionTicket))];
+    uint256 requestedShares = _exitRequests[keccak256(abi.encode(receiver, positionTicket, _nounce))];

   function claimExitedAssets(
     uint256 positionTicket,
     uint256 exitQueueIndex,
+    uint256 _nounce
   ) external returns (uint256 newPositionTicket, uint256 claimedShares, uint256 claimedAssets);
-    bytes32 queueId = keccak256(abi.encode(msg.sender, positionTicket));
+    bytes32 queueId = keccak256(abi.encode(msg.sender, positionTicket, _nounce));

     if (leftShares > 1) {
       // update user's queue position
       newPositionTicket = positionTicket + claimedShares;
-      _exitRequests[keccak256(abi.encode(msg.sender, newPositionTicket))] = leftShares;
+      _exitRequests[keccak256(abi.encode(msg.sender, newPositionTicket, _nounce))] = leftShares;

In this way, the scenario will be:

State A:

_exitQueue.getLatestTotalTickets() = 10_000
queuedShares = 0

User A enterExitQueue witH 6_000 shares So

_exitQueue.getLatestTotalTickets() = 10_000
queuedShares = 6_000
User A positionTicket = 10_000
nounce = 0
_exitRequests[keccak256(abi.encode(User A,positionTicket, nounce))] = 6_000

State B:
_exitQueue.getLatestTotalTickets() = 13_000
queuedShares = 3_000
User A New positionTicket = 13_000
_nounce = 0
_exitRequests[keccak256(abi.encode(User A, positionTicket, nounce))] = 3_000

User A enterExitQueue again with 1_000 share So

State C:
_exitQueue.getLatestTotalTickets() = 13_000
queuedShares = 4_000
User A positionTicket = 13_000
nounce = 1
_exitRequests[keccak256(abi.encode(User A, positionTicket, nounce))] = 1_000

As you see User A's positionTicket hasn't changed.

hats-finance / StakeWise-0xd91cd6ed6c9a112fdc112b1a3c66e47697f522cd

users will lose share if they call enterExitQueue multiple times. #95