search
Active Boosts

57635 sc critical erc4626 share transfers desynchronize time lock ledger blocking standard withdrawals for recipients

Submitted on Oct 27th 2025 at 19:29:38 UTC by @Rhaydden for Audit Comp | Belongarrow-up-right

  • Report ID: #57635

  • Report Type: Smart Contract

  • Report severity: Critical

  • Target: https://github.com/immunefi-team/audit-comp-belong/blob/main/contracts/v2/periphery/Staking.sol

  • Impacts:

    • Griefing (e.g. no profit motive for an attacker, but damage to the users or the protocol)

hashtag
Description

hashtag
Issue description

The Staking vault inherits Solady ERC4626 (transferable by default) and implements a time-lock by keeping a per-user ledger:

/// @notice User stake entries stored as arrays per staker.
/// @dev Public getter: stakes(user, i) → (shares, timestamp).
mapping(address staker => Stake[] times) public stakes;

Lock entries are created only on deposit (mint):

function _deposit(address by, address to, uint256 assets, uint256 shares) internal override {
    super._deposit(by, to, assets, shares);
    // lock freshly minted shares
    stakes[to].push(Stake({shares: shares, timestamp: block.timestamp}));
}

Withdrawals require consuming unlocked shares from stakes[_owner] and will revert if not enough unlocked shares exist:

The unlocked-only consumption routine:

Solady’s ERC20 implementation calls transfer hooks (_beforeTokenTransfer / _afterTokenTransfer) that are available to override, but Staking does not override them to maintain stakes consistency when shares are transferred:

Because stakes is not mirrored on transfers, transferring shares increases the recipient’s ERC4626 balance but leaves stakes[recipient] empty. As a result, standard withdraw/redeem calls revert for recipients with transferred shares (they have shares but no unlocked stake entries). Recipients must either re-transfer the shares back or use the emergency redemption path (which imposes a penalty).

hashtag
Impact

Medium — Griefing

  • Users receiving transferred shares cannot use standard withdraw/redeem. They must either re-transfer the shares (inconvenience and possible extra fees) or use emergencyRedeem which applies a penalty.

hashtag
Recommended mitigation steps

Two main approaches:

  • Restrict transfers:

    • Override ERC20._beforeTokenTransfer in Staking to revert when from != address(0) and to != address(0) (i.e., disallow transfers between accounts while preserving mint/burn).

  • Mirror locks on transfers:

    • Override ERC20._beforeTokenTransfer or _afterTokenTransfer to handle from != address(0) && to != address(0):

      • Debit amount from stakes[from] using oldest-first order, splitting entries as needed.

      • Push equivalent entries to stakes[to], preserving original timestamps.

      • Ensure mints/burns are handled so _deposit does not double-record entries.

    • Deterministic ordering prevents “lock bleaching.”

Do not add behavior changes beyond mirroring or transfer restrictions. Preserve timestamps when mirroring to avoid bypassing min-stake periods.

hashtag
Proof of Concept

The following PoC demonstrates the issue: transferred shares appear in ERC4626 views (e.g., maxRedeem) but runtime lock checks revert redeem/withdraw for the recipient because their stakes array is empty. emergencyRedeem still succeeds (but with a penalty).

Use the provided test & config snippets below to reproduce locally.

1

hashtag
PoC test file

Create test/v2/periphery/staking.transfer-lock.poc.test.ts with the following content:

2

hashtag
Hardhat config adjustments (optional)

Because of compatibility issues, the following changes were made to hardhat.config.ts. This file is provided for context — keep links and settings unchanged if you reuse it.

3

hashtag
How to run the PoC

Run the test with:

Expected test result:

hashtag
Additional notes

  • Do not modify external links or URLs from the original report.

  • Two mitigation patterns are presented; choose the one that matches product requirements:

    • Disallow transfers if transfers are not intended.

    • Mirror stake entries on transfer if token transferability must be preserved.

If you want, I can generate a suggested patch for Staking.sol implementing either (A) a transfer restriction via _beforeTokenTransfer that reverts on non-mint/burn transfers, or (B) an implementation that mirrors stakes on transfers (debiting from sender's stakes oldest-first and crediting the recipient with preserved timestamps). Which option would you like to see implemented as a code patch?

Previous57594 sc medium signature collision from abi encodepacked adjacent strings enables unauthorized nft actions mint uri abuse Next57454 sc low referral percentages schedule stuck on first configuration

Was this helpful?