Tori Finance

trUSD

6.1 Blacklisted users can retrieve silo funds via unstake(), bypassing blacklist restrictions

The unstake() function does not check whether the caller has the FULL_RESTRICTED_STAKER_ROLE before releasing funds from the TrUsdSilo. When a user initiates a cooldown via cooldownAssets(...) or unstake(), their strUSD shares are burned and the underlying TrUSD is transferred to the TrUsdSilo contract. If the user is subsequently blacklisted, they can still call unstake(...) after the cooldown expires to withdraw the TrUSD to any receiver address.

The blacklist system correctly blocks unstake(...), _deposit, _withdraw(), and _update()for restricted users, but unstake() was not included in these protections. This creates an inconsistency where the blacklist is enforced across all vault operations except the final silo withdrawal step.

Critically, there is no admin mechanism to seize assets already in the silo. The redistributeLockedAmount(...) function only operates on strUSD share balances, but the blacklisted user's shares were already burned when they initiated the cooldown. The TrUsdSilo contract itself has no admin functions — its withdraw(...) is callable only by StakedTrUSD, and only through the unstake() or unstake() code paths. The rescueTokens(...) function on StakedTrUSD cannot reach the silo since it is a separate contract.

Additionally, a user who observes an addToBlacklist(...) transaction in the mempool can front-run it by calling cooldownAssets(maxWithdraw()) to move all their assets into the silo before the blacklist takes effect, guaranteeing an escape path for the full balance.

function unstake(address receiver) external {
    StakedTrUSDStorage storage $ = _getStakedTrUSDStorage();
    UserCooldown storage userCooldown = $.cooldowns[msg.sender];
    uint256 assets = userCooldown.underlyingAmount;

    // @audit-issue No FULL_RESTRICTED_STAKER_ROLE check — blacklisted users can withdraw from silo.
    if (block.timestamp >= userCooldown.cooldownEnd || $.cooldownDuration == 0) {
        userCooldown.cooldownEnd = 0;
        userCooldown.underlyingAmount = 0;
        $.silo.withdraw(receiver, assets);
    } else {
        revert InvalidCooldown();
    }
}

function cancelCooldown() external nonReentrant {
    // ....
    // @audit Blacklist IS checked here, but NOT in unstake().
    if (hasRole(FULL_RESTRICTED_STAKER_ROLE, msg.sender)) {
        revert OperationNotAllowed();
    }
    // ....

Recommendation

Consider adding a FULL_RESTRICTED_STAKER_ROLE check to the unstake(...) function that reverts if the caller is blacklisted, consistent with the protections already present in unstake(...), _deposit(...), _withdraw(...), and _update(...). Additionally, consider adding an admin function that can redirect a blacklisted user's silo cooldown assets to a designated address, similar to how redistributeLockedAmount(...) handles strUSD shares. This ensures the admin has recourse for assets that entered the silo before the user was blacklisted.

6.2 StakingRewardsDistributor STAKING_VAULT cannot be updated in cases of compromise or if desired.

StakingRewardsDistributor contract has functions to revoke and update the approval amount for a staking vault in cases where staking vault is compromised or needs updating.

The comments in the file clearly shows that the protocol anticipates such scenarios. But the STAKING_VAULT address is defined as an immutable value (set only in constructor), meaning it will not be updated in cases that requires it. This means for any compromise, StakingRewardsDistributor will need to be deployed again with a new STAKING_VAULT address.

Recommendation

Consider making the STAKING_VAULT address updatable with a setter function.

Resolution

Acknowledged.

6.3 ToriTimelock delay bounds only enforced at construction, bypassable via inherited updateDelay

ToriTimelock defines MIN_DELAY (1 day) and MAX_DELAY (30 days) constants and enforces them in the constructor. However, the contract inherits TimelockController from OpenZeppelin, which exposes an updateDelay(...) function that allows the timelock itself (via self-administered governance proposals) to set the delay to any value, including 0.

The updateDelay function requires msg.sender == address(this), meaning it must be called through the timelock\'s schedule-and-execute flow. Since the timelockAdmin is both proposer and executor, they can propose a call to updateDelay(0), wait the current delay period, and execute it. After execution, the timelock's delay becomes 0, allowing all subsequent proposals to be executed immediately without any waiting period.

The MIN_DELAY and MAX_DELAY constants create a false sense of security by suggesting that delay bounds are always enforced. ToriTimelock does not override updateDelay(...) to enforce these bounds, so they are only checked once at deployment and never again.

contract ToriTimelock is TimelockController {
    uint256 public constant MIN_DELAY = 1 days;
    uint256 public constant MAX_DELAY = 30 days;

    constructor(
        uint256 minDelay,
        address timelockAdmin,
        address canceller
    ) TimelockController(
        minDelay,
        _toArray(timelockAdmin),
        _toArray(timelockAdmin),
        address(this)
    ) {
        require(minDelay >= MIN_DELAY, "Delay too short");
        require(minDelay <= MAX_DELAY, "Delay too long");
    }

Recommendation

Consider overriding the updateDelay(...) function in ToriTimelock to enforce the MIN_DELAY and MAX_DELAY bounds. The override should validate that any new delay value falls within the defined range before calling the parent implementation. This ensures the delay bounds remain enforced throughout the contract's lifetime, not just at deployment.

6.4 TrUsdSilo lacks withdrawal for other assets

TrUsdSilo stores TrUSD assets during cooldown period and staking vault to withdraw it. But the downside is that if any other asset is sent to the silo, it is lost as there is no way to withdraw the assets.

Recommendation

Consider making the withdrawal function flexible, so that other assets can be withdrawn.

6.5 removeAllowedToken cannot remove disabled tokens, creating permanently orphaned storage entries

ToriMinting's removeAllowedToken() requires tokenConfig[_token].isActive == true before deleting the token config, but StakingRewardsDistributor sets isActive to false. This means the natural admin workflow of disable-then-remove always reverts, because the removal guard rejects any token that has already been disabled.

The workaround is to re-enable the token via enableAllowedToken() before removing it, but this requires the TIMELOCK_ADMIN_ROLE — a different and more privileged role than the DEFAULT_ADMIN_ROLE used for both disableAllowedToken and removeAllowedToken. The DEFAULT_ADMIN who disabled the token cannot independently complete the removal without coordinating with the timelock admin.

A more severe edge case creates a permanently irrecoverable state. If the DEFAULT_ADMIN calls setMaxMintPerBlock with a value of 0 for an active token (there is no zero-value guard in that function), then calls disableAllowedToken, the token enters a state where isActive is false and maxMintPerBlock is 0. In this state, removeAllowedToken reverts because isActive is false, and enableAllowedToken reverts because maxMintPerBlock is 0. No function can transition the token out of this state. The only escape hatch is for the TIMELOCK_ADMIN to call addAllowedToken to overwrite the orphaned entry with a fresh active configuration, but this re-adds the token rather than removing it.

The isActive guard in removeAllowedToken conflates "token exists" with "token is currently active." The function's intent is to delete a known token config, but the guard prevents deletion of any token that is not in the active state, which directly contradicts the removal workflow.

function removeAllowedToken(address _token) external onlyRole(DEFAULT_ADMIN_ROLE) {
    if (_token == address(0)) revert ZeroAddress();
    if (!tokenConfig[_token].isActive) revert TokenNotAllowed(_token);
    delete tokenConfig[_token];
    emit TokenRemoved(_token);
}

function disableAllowedToken(address _token) external onlyRole(DEFAULT_ADMIN_ROLE) {
    if (_token == address(0)) revert ZeroAddress();
    if (!tokenConfig[_token].isActive) revert TokenNotAllowed(_token);
    tokenConfig[_token].isActive = false;
}

Recommendation

Consider changing the existence check in removeAllowedToken (...) so that it verifies the token config entry exists in storage (for example, by checking that at least one struct field has a non-default value) rather than requiring isActive to be true. This allows removal of tokens regardless of whether they are currently active or disabled, which aligns with the function's intended purpose of cleaning up token configurations.

6.6 CannotRescueRewardToken error declared but unused; rescueTokens allows owner to extract TrUSD

StakingRewardsDistributor declares error CannotRescueRewardToken() at line 55 but never uses it. The rescueTokens(...) function performs no check against the TRUSD_TOKEN immutable, allowing the owner to call rescueTokens(address(TRUSD_TOKEN), to, amount) and extract the contract's operational TrUSD balance.

The analogous function in StakedTrUSD guards the underlying asset with if (address(token) == asset()) revert InvalidToken(), preventing the admin from draining the vault's backing token. The declared but unused error in StakingRewardsDistributor indicates the developers intended to implement the same safeguard but did not wire it up.

The contract is designed to hold TrUSD between funding events and distribution to the staking vault via transferInRewards(...). Without the guard, the owner (multisig) can bypass the normal transferInRewards(...) flow — which has operator restrictions, amount checks, and event logging — and extract TrUSD directly through the rescue mechanism. Both the solidity-auditor and sc-auditor AI tools independently identified this issue.

error CannotRescueRewardToken();

function rescueTokens(address _token, address _to, uint256 _amount) external nonReentrant onlyOwner {
    if (_token == address(0)) revert InvalidZeroAddress();
    if (_to == address(0)) revert InvalidZeroAddress();
    if (_amount == 0) revert InvalidAmount();

    // Contract can only receive ETH via selfdestruct, allow rescuing it
    if (_token == _ETH_ADDRESS) {
        (bool success,) = _to.call{ value: _amount }("");
        if (!success) revert TransferFailed();
    } else {
        IERC20(_token).safeTransfer(_to, _amount);
    }
    emit TokensRescued(_token, _to, _amount);
}

Recommendation

Consider adding a guard to rescueTokens(...) that reverts with CannotRescueRewardToken when the token parameter equals address(TRUSD_TOKEN). This prevents accidental or intentional extraction of the contract's operational TrUSD through the rescue mechanism, consistent with the pattern already implemented in StakedTrUSD.rescueTokens(...).

6.7 Cross-chain blacklist systems are independent with no synchronization mechanism

The protocol uses two independent blacklist systems across its cross-chain deployment with no synchronization mechanism:

1. StakedTrUSD (home chain): Uses AccessControl with FULL_RESTRICTED_STAKER_ROLE ;
2. StakedTrUSDOFT (remote chains): Uses a local mapping(address => bool) managed by a separate blackLister address ;

Blacklisting a user on the home chain does not propagate to remote chains. A user restricted on the home chain can still hold, transfer, and receive StakedTrUSDOFT tokens on any remote chain where they have not been independently blacklisted. Similarly, a user blacklisted on one remote chain remains unrestricted on all other chains.

The operational burden of maintaining consistent blacklists across all deployed chains creates a window during which restricted users can move assets cross-chain before administrators can propagate the restriction.

mapping(address => bool) public blackList;

bytes32 private constant FULL_RESTRICTED_STAKER_ROLE = keccak256("FULL_RESTRICTED_STAKER_ROLE");

Recommendation

Consider documenting the operational requirement for administrators to maintain blacklists on all deployed chains simultaneously. Consider implementing a cross-chain blacklist synchronization message via LayerZero so that blacklisting on the home chain automatically propagates to remote chains.

Resolution

This is a known design trade-off. Our off-chain services will maintain blacklists through a periodic synchronization process, ensuring consistency is enforced at the operational level.

6.8 Repeated cooldownAssets/cooldownShares calls reset the cooldown timer for all accumulated assets

In StakedTrUSD, both cooldownAssets() and unstake(...) accumulate underlyingAmount via += but overwrite cooldownEnd(...) with a fresh timestamp on every call. If a user calls cooldownAssets(...) a second time before the first cooldown has matured, the timer resets for all accumulated assets — including those from the first call that may have been close to maturity.

For example, with a 7-day cooldown: a user who calls cooldownAssets(100) at T=0 and then cooldownAssets(50) at T=6d will have cooldownEnd(...) reset to T+13d for all 150 assets. The first 100 assets that were 6 days into their 7-day cooldown are now locked for an additional 7 days.

This can cause users to accidentally extend their own cooldown period without realizing it. The workaround is to call unstake() for matured assets before starting a new cooldown.

This is a standard design pattern inherited from Ethena's sUSDe (StakedUSDe), chosen for storage simplicity over per-cooldown tracking.

$.cooldowns[msg.sender].cooldownEnd = uint104(block.timestamp) + $.cooldownDuration;
$.cooldowns[msg.sender].underlyingAmount += assets;

Recommendation

Document the cooldown timer reset behavior clearly in NatSpec and user-facing documentation. Consider adding a view function that lets users check their current cooldown state before initiating a new one, reducing the risk of accidental self-griefing.

Resolution

This is an intentional design choice.

6.9 StakedTrUSD ERC-4626 max view functions return incorrect values under access restrictions

StakedTrUSD does not override the ERC-4626 maxWithdraw, maxRedeem, maxDeposit, or maxMint view functions to account for its custom access restrictions. This violates several ERC-4626 MUST requirements.

When cooldownDuration is non-zero (the default configuration of 7 days), the withdraw() and redeem() functions revert via the modifier. However, maxWithdraw(...) and maxRedeem(...) still return non-zero values based on the owner's share balance because the inherited OpenZeppelin ERC4626Upgradeable implementation is used without modification. ERC-4626 mandates that maxWithdraw(...) and maxRedeem(...) MUST return the maximum amount that would not cause a revert. Since the standard withdrawal path reverts when cooldown is active, both functions should return 0. Users must use the cooldownAssets(...)/cooldownShares(...) flow instead, but integrating protocols and aggregators checking maxWithdraw(...)/maxRedeem(...) will receive incorrect values, attempt withdrawals, and have transactions revert.

Addresses with the FULL_RESTRICTED_STAKER_ROLE are blocked from depositing in the _deposit override, which reverts with OperationNotAllowed. However, maxDeposit(...) and maxMint(...) return type(uint256).max for these addresses. ERC-4626 requires these functions to return 0 when deposit or mint would revert for the given receiver.

function withdraw(uint256 assets, address receiver, address _owner)
    public
    virtual
    override
    ensureCooldownOff
    returns (uint256)
{
    return super.withdraw(assets, receiver, _owner);
}

function _deposit(address caller, address receiver, uint256 assets, uint256 shares)
    internal
    override
    nonReentrant
    notZero(assets)
    notZero(shares)
{
    if (hasRole(FULL_RESTRICTED_STAKER_ROLE, caller) || hasRole(FULL_RESTRICTED_STAKER_ROLE, receiver)) {
        revert OperationNotAllowed();
    }
    super._deposit(caller, receiver, assets, shares);
    _checkMinShares();
}

Recommendation

Consider overriding maxWithdraw(...) and maxRedeem(...) in StakedTrUSD to return 0 when cooldownDuration is non-zero, since the standard withdrawal path is disabled in that mode. Additionally, consider overriding maxDeposit(...) and maxMint(...) to return 0 when the receiver has the FULL_RESTRICTED_STAKER_ROLE. This ensures ERC-4626 compliance and correct composability with external protocols that rely on these view functions to determine deposit and withdrawal limits.

Resolution

Acknowledged.

6.10 cancelCooldown() allows zero-share minting causing silent loss of user funds

The cancelCooldown() function calls _mint(msg.sender, shares) directly without checking if shares == 0. When previewDeposit(assets) returns 0 due to Floor rounding (which can occur with dust amounts or extreme exchange rates), the function succeeds without reverting, causing the user to permanently lose their cooldown assets while the vault is silently enriched.

function cancelCooldown() external nonReentrant {
        // ...
        // ...

        uint256 shares = previewDeposit(assets);

        // Transfer assets from Silo back to vault
        $.silo.withdraw(address(this), assets);

        // Mint shares
        _mint(msg.sender, shares);

        // Ensure MIN_SHARES invariant
        _checkMinShares();

        emit CooldownCancelled(msg.sender, assets, shares);
    }

Recommendation

Consider checking shares minted is not zero.

6.11 redistributeLockedAmount() violates MIN_SHARES invariant

The redistributeLockedAmount() function burns shares from a blacklisted user and optionally mints to a recipient (or burns to address(0) to distribute value to remaining stakers). However, it lacks a _checkMinShares(...) call after the burn/mint operations.

function redistributeLockedAmount(address from, address to) external nonReentrant onlyRole(DEFAULT_ADMIN_ROLE) {
        if (hasRole(FULL_RESTRICTED_STAKER_ROLE, from) && !hasRole(FULL_RESTRICTED_STAKER_ROLE, to)) {
            uint256 amountToDistribute = balanceOf(from);

            _burn(from, amountToDistribute);

            if (to != address(0)) {
                _mint(to, amountToDistribute);
            }
            // When to == address(0): assets remain in vault, share value increases instantly for remaining stakers

            emit LockedAmountRedistributed(from, to, amountToDistribute);
        } else {
            revert OperationNotAllowed();
        }
    }

This allows the vault to be left in an invalid state where 0 < totalSupply() < MIN_SHARES (1e18), which causes all subsequent deposits and withdrawals to revert due to the MIN_SHARES invariant check.This issue can only be triggered by admin redistributing small blacklisted amounts to address(0).

Recommendation

Consider adding a the minimum shares check after the burn and mint operations.

6.12 reportLoss silently burns partial amount instead of reverting with declared InsufficientAssets error

IStakedTrUSD declares an InsufficientAssets() error with the NatSpec comment 'Error emitted when trying to report loss greater than available assets', indicating that reportLoss should revert when the loss exceeds the vault's balance. However, the implementation in StakedTrUSD.reportLoss() silently handles this case by burning only the available balance instead of reverting.

When the computed totalBurn (loss amount plus cancelled unvested rewards) exceeds the vault's token balance, the function burns only what is available and emits LossReported with the available amount rather than the requested amount. The upstream StakingRewardsDistributor.reportLoss(...) then emits its own LossReported event with the originally requested amount, creating a discrepancy in the event trail between the requested and actual loss.

uint256 available = IERC20(asset()).balanceOf(address(this));

if (available >= totalBurn) {
    IERC20Burnable(asset()).burn(totalBurn);
    emit LossReported(amount);
} else {
    IERC20Burnable(asset()).burn(available);
    emit LossReported(available);
}

/// @notice Error emitted when trying to report loss greater than available assets
error InsufficientAssets();

Recommendation

Consider either using the declared InsufficientAssets error to revert when the loss exceeds the vault's available balance, consistent with the interface's documented intent, or remove the unused error from the interface if the silent partial-burn behavior is intentional. If keeping the partial-burn approach, consider returning the actual burned amount to callers like StakingRewardsDistributor.

6.13 verifyNonce truncates uint256 nonce to uint128, reducing effective nonce space

In ToriMinting, verifyNonce, the nonce parameter is declared as uint256 in both the Order struct and the EIP-712 type hash, but the bitmap slot computation truncates it to uint128 before shifting. Specifically, uint128(nonce) >> 8 discards the upper 128 bits (bits 128-255) of the nonce entirely. This means two distinct uint256 nonces that share the same lower 128 bits will map to the same bitmap slot and bit position, causing the second order to revert with InvalidNonce even though it has a unique nonce from the EIP-712 signature perspective.

The nonce is generated randomly off-chain rather than sequentially, which increases the practical relevance of the truncation. With random 256-bit nonces, the upper 128 bits are discarded silently, so two nonces that differ only in those bits will collide. The effective nonce space is reduced from 2^256 to 2^128 (2^120 slots × 256 bit positions per slot). While 2^128 is still an enormous space — birthday collisions require approximately 2^64 nonces which is practically infeasible — the mismatch between the declared uint256 type and the 128-bit effective space creates a correctness issue. Off-chain systems that generate nonces using the full 256-bit range (e.g., combining chain IDs, timestamps, or counters in upper bits) could inadvertently produce collisions in the lower 128 bits, leading to silent order rejections that are difficult to diagnose.

function verifyNonce(address sender, uint256 nonce) public view returns (uint256, uint256, uint256) {
    if (nonce == 0) revert InvalidNonce();
    // @audit-issue Nonce is truncated from uint256 to uint128, discarding upper 128 bits.
    uint256 invalidatorSlot = uint128(nonce) >> 8;
    uint256 invalidatorBit = 1 << uint8(nonce);
    uint256 invalidator = _orderBitmaps[sender][invalidatorSlot];
    if (invalidator & invalidatorBit != 0) revert InvalidNonce();

    return (invalidatorSlot, invalidator, invalidatorBit);
}

Recommendation

Consider using a sequential nonce scheme instead of random generation, starting from 1, the truncation to uint128 is harmless for the first 2^128 nonces per sender, which is practically inexhaustible. As a contract-level alternative, use the full uint256 nonce for the bitmap slot computation instead of truncating to uint128.

Resolution

Acknowledged

6.14 verifyStablesLimit divides by trusdAmount before verifyOrder's zero-amount guard, causing uninformative panic

In ToriMinting verifyOrder(...), the call to verifyStablesLimit(...) precedes the zero-amount guard (if (order.collateral_amount == 0 || order.trusd_amount == 0) revert InvalidAmount()). Inside verifyStablesLimit(...) computes (difference * STABLES_RATIO_MULTIPLIER) / trusdAmount. When TrUSD_amount is 0, this causes a Solidity Panic(0x12) — division by zero — before the code reaches the intended InvalidAmount revert.

Both code paths revert and no state change occurs, so there is no fund risk. However, the Panic error is uninformative compared to the intended InvalidAmountcustom error. Off-chain systems parsing revert reasons will see an opaque arithmetic panic rather than the descriptive application error.

The modifiers belowMaxMintPerBlock and belowGlobalMaxMintPerBlock do not block zero amounts — the check current + 0 > max passes for any non-exceeded block — so there is no earlier guard preventing this path.

if (!verifyStablesLimit(order.collateral_amount, order.trusd_amount, order.collateral_asset, order.order_type))
{
    revert InvalidStablePrice();
}

if (order.collateral_amount == 0 || order.trusd_amount == 0) revert InvalidAmount();

uint128 differenceInBps = (difference * STABLES_RATIO_MULTIPLIER) / trusdAmount;

Recommendation

Consider moving the zero-amount guard above the verifyStablesLimit(...) call so that orders with TrUSD_amount=0 or collateral_amount=0 are rejected with the descriptive InvalidAmount error before any arithmetic is performed on those values.