hats-finance / Accumulated-finance-0x75278bcc0fa7c9e3af98654bce195eaf3bb6a784

0 stars 0 forks source link

Lack of Finalization for Undelegation Process will Lock Tokens Indefinitely #33

Open hats-bug-reporter[bot] opened 2 weeks ago

hats-bug-reporter[bot] commented 2 weeks ago

Github username: @4gontuk Twitter username: 4gontuk Submission hash (on-chain): 0xab2e7b788017f364bf8feed7596545a4872df0da43edd94d0af0a01d7f617142 Severity: low

Description:

Description:

The stROSEMinter contract is designed to handle staking and undelegation processes on the Oasis Network. It interacts with the consensus layer through the Subcall library to delegate and undelegate tokens. The key functions involved in the undelegation process are undelegate() and takeReceiptUndelegate().

However, the contract lacks a standard function to finalize the undelegation process and claim the undelegated tokens after the debonding period. The emergencyTakeReceiptUndelegateDone() function exists but is intended for emergency use and is not integrated into the normal flow. This oversight can lead to indefinite locking of tokens, as there is no mechanism to claim the undelegated tokens after the debonding period.

The root cause of the issue is the absence of a function to finalize the undelegation process and retrieve the undelegated tokens. This can be seen in the following code snippets:

// Initiates the undelegation process
function undelegate(StakingAddress from, uint128 shares) public onlyOwner {
    uint64 receiptId = nextReceiptId++;
    Subcall.consensusUndelegate(from, shares, receiptId);
    undelegationReceipts[receiptId] = UndelegationReceipt({
        exists: true,
        from: from,
        blockNumber: block.number,
        receiptTaken: false,
        receiptTakenBlockNumber: 0,
        shares: shares,
        epoch: 0,
        endReceiptId: 0
    });
    emit Undelegate(from, shares, receiptId);
}

// Processes the start of the undelegation process
function takeReceiptUndelegate(uint64 receiptId) public onlyOwner {
    UndelegationReceipt storage receipt = undelegationReceipts[receiptId];
    require(block.number > receipt.blockNumber, "ReceiptNotReady");
    require(receipt.exists, "ReceiptNotExists");
    require(receipt.receiptTaken == false, "ReceiptAlreadyTaken");
    (uint64 epoch, uint64 endReceiptId) = Subcall.consensusTakeReceiptUndelegateStart(receiptId);
    receipt.receiptTaken = true;
    receipt.receiptTakenBlockNumber = block.number;
    receipt.epoch = epoch;
    receipt.endReceiptId = endReceiptId;
    emit TakeReceiptUndelegate(receiptId, epoch, endReceiptId);
}

The highest impact scenario is that users who attempt to undelegate their tokens will find that they cannot actually retrieve them, leading to a loss of funds and breaking the core functionality of the staking system.

Impact

The lack of a standard mechanism to finalize the undelegation process and claim the undelegated tokens after the debonding period can lead to indefinite locking of tokens. This effectively prevents users from ever recovering their undelegated tokens

Attack Scenario:

  1. The owner calls undelegate() to initiate the undelegation process.
  2. The owner calls takeReceiptUndelegate() to process the start of the undelegation process.
  3. The debonding period passes, but there is no function to finalize the undelegation and claim the tokens.
  4. The tokens remain locked indefinitely as there is no mechanism to retrieve them.

Proof of Concept

  1. The owner calls undelegate(from, shares) to initiate the undelegation process:

    function undelegate(StakingAddress from, uint128 shares) public onlyOwner {
        uint64 receiptId = nextReceiptId++;
        Subcall.consensusUndelegate(from, shares, receiptId);
        undelegationReceipts[receiptId] = UndelegationReceipt({
            exists: true,
            from: from,
            blockNumber: block.number,
            receiptTaken: false,
            receiptTakenBlockNumber: 0,
            shares: shares,
            epoch: 0,
            endReceiptId: 0
        });
        emit Undelegate(from, shares, receiptId);
    }
  2. The owner calls takeReceiptUndelegate(receiptId) to process the start of the undelegation process:

    function takeReceiptUndelegate(uint64 receiptId) public onlyOwner {
        UndelegationReceipt storage receipt = undelegationReceipts[receiptId];
        require(block.number > receipt.blockNumber, "ReceiptNotReady");
        require(receipt.exists, "ReceiptNotExists");
        require(receipt.receiptTaken == false, "ReceiptAlreadyTaken");
        (uint64 epoch, uint64 endReceiptId) = Subcall.consensusTakeReceiptUndelegateStart(receiptId);
        receipt.receiptTaken = true;
        receipt.receiptTakenBlockNumber = block.number;
        receipt.epoch = epoch;
        receipt.endReceiptId = endReceiptId;
        emit TakeReceiptUndelegate(receiptId, epoch, endReceiptId);
    }
  3. The debonding period passes, but there is no function to finalize the undelegation and claim the tokens.

  4. The tokens remain locked indefinitely as there is no mechanism to retrieve them.

The absence of a function to finalize the undelegation process and claim the undelegated tokens after the debonding period can lead to indefinite locking of tokens, effectively preventing users from ever recovering their undelegated tokens and leading to significant loss of funds.

ilzheev commented 2 weeks ago

Finalization is not needed: https://github.com/hats-finance/Accumulated-finance-0x75278bcc0fa7c9e3af98654bce195eaf3bb6a784/issues/6#issuecomment-2327148488