berndartmueller - Entitled asset shares are not withdrawn and are lost when minting rollovers

[sherlock-admin]

berndartmueller

high

Entitled asset shares are not withdrawn and are lost when minting rollovers

Summary

The entitled pro-rata shares of the counterparty vault's TVL are not withdrawn nor credited to the user when rolling over to the next epoch.

Vulnerability Detail

The Carousel.mintRollovers function iterates over all queued rollover items and checks the entitled shares for the current item and the epoch. If the entitled shares (entitledShares) are greater than the deposited assets (queue[index].assets), the user is considered to have "won" the epoch, and the rollover is minted. All shares are then burned.

The shares for the rolled-over epoch (_epochId) are calculated in line 436:

uint256 assetsToMint = queue[index].assets - relayerFee;

This effectively mints the same amount of deposited assets minus the relayer fee as shares for the next epoch. However, the "won" assets from the resolved epoch are not withdrawn nor are they credited to the assetsToMint.

Impact

A user who has won an epoch and is entitled to pro-rata shares of the counterparty vault's TVL will lose those assets when rolled over to the next epoch.

Code Snippet

src/v2/Carousel/Carousel.mintRollovers

361: function mintRollovers(uint256 _epochId, uint256 _operations)
362:     external
363:     epochIdExists(_epochId)
364:     epochHasNotStarted(_epochId)
365:     nonReentrant
366: {
...      // [...]
392:
393:     while ((index - prevIndex) < (_operations)) {
394:         // only roll over if last epoch is resolved
395:         if (epochResolved[queue[index].epochId]) {
396:             uint256 entitledShares = previewWithdraw(
397:                 queue[index].epochId,
398:                 queue[index].assets
399:             );
400:             // mint only if user won epoch he is rolling over
401:             if (entitledShares > queue[index].assets) {
402:                 // skip the rollover for the user if the assets cannot cover the relayer fee instead of revert.
403:                 if (queue[index].assets < relayerFee) {
404:                     index++;
405:                     continue;
406:                 }
407:                 // @note we know shares were locked up to this point
408: @>              _burn(
409: @>                  queue[index].receiver,
410: @>                  queue[index].epochId,
411: @>                  queue[index].assets
412: @>              );
413:                 // transfer emission tokens out of contract otherwise user could not access them as vault shares are burned
414:                 _burnEmissions(
415:                     queue[index].receiver,
416:                     queue[index].epochId,
417:                     queue[index].assets
418:                 );
419:                 // @note emission token is a known token which has no before transfer hooks which makes transfer safer
420:                 emissionsToken.safeTransfer(
421:                     queue[index].receiver,
422:                     previewEmissionsWithdraw(
423:                         queue[index].epochId,
424:                         queue[index].assets
425:                     )
426:                 );
427:
428:                 emit Withdraw(
429:                     msg.sender,
430:                     queue[index].receiver,
431:                     queue[index].receiver,
432:                     _epochId,
433:                     queue[index].assets,
434:                     entitledShares
435:                 );
436:                 uint256 assetsToMint = queue[index].assets - relayerFee;
437:                 _mintShares(queue[index].receiver, _epochId, assetsToMint);
438:                 emit Deposit(
439:                     msg.sender,
440:                     queue[index].receiver,
441:                     _epochId,
442:                     assetsToMint
443:                 );
444:                 rolloverQueue[index].assets = assetsToMint;
445:                 rolloverQueue[index].epochId = _epochId;
446:                 // only pay relayer for successful mints
447:                 executions++;
448:             }
449:         }
450:         index++;
451:     }
452:
453:     if (executions > 0) rolloverAccounting[_epochId] = index;
454:
455:     if (executions * relayerFee > 0)
456:         asset.safeTransfer(msg.sender, executions * relayerFee);
457:
458:     emit RelayerMinted(_epochId, executions);
459: }

Tool used

Manual Review

Recommendation

Consider either withdrawing and sending the entitled and "won" assets to the user before burning the queue[index].assets shares or mint the "won" assets as part of the rollover to the next epoch by adjusting the assetsToMint calculation in line 436:

uint256 assetsToMint = entitledShares - relayerFee;

Duplicate of #163