Deq Liquid Staking Protocol Security Review Report

The burn() function of the AvailWithdrawalHelper contract does not consider the current ratio between shares and assets. Consequently, when a protocol incurs losses and the updateAssets() function of StakedAvail is invoked with a negative delta, the burn() call either reverts or distributes losses unfairly among users trying to withdraw their assets.

Consider the the following scenario where the burn() call reverts:

1. Alice deposits 1000 Avail tokens into the protocol (via mint()) and receives 1000 stAvail tokens.
2. Subsequently, Alice calls the burn() function to burn all her shares (1,000 stAvail tokens).
3. The UPDATER_ROLE then triggers the updateAssets() function with a negative delta value (e.g., -100), resulting in the asset value being updated to 900 post-execution of the function.
4. Alice proceeds to call the burn() function in the AvailWithdrawalHelper contract, subsequently invoking the stAvail.updateAssetsFromWithdrawals(amount, shares) function with the values amount=1000 and shares=1000.
5. In the StakedAvail contract, the assets value is 900, but the amount passed from the AvailWithdrawalHelper is 1000. This discrepancy causes the transaction to revert with an underflow error.

In another scenario, a malicious user could front-run the updateAssets() call by creating a withdrawal for their Avail tokens, thus avoiding paying for the losses. Less fortunate users would then have to share those additional losses of the malicious user.

function burn(uint256 id) external {
    uint256 prevWithdrawalAccAmt = withdrawals[id - 1].accAmount;
    Withdrawal memory withdrawal = withdrawals[id];
    uint256 amount = withdrawal.accAmount - prevWithdrawalAccAmt;
    if (lastFulfillment < id) {
        // increment lastFulfillment to id
        _fulfill(id);
    }
    remainingFulfillment -= amount;
    address owner = ownerOf(id);
    withdrawalAmount -= amount;
    _burn(id);
    stAvail.updateAssetsFromWithdrawals(amount, withdrawal.shares);
    avail.safeTransfer(owner, amount);
}

The burn() function of AvailWithdrawalHelper should utilize the current shares-to-assets ratio.

Although the DeqRouter.sol contract's swap functions incorporate slippage protection, they currently lack a deadline element. This absence leaves transactions without a specified deadline, potentially allowing them to be included at any time by validators while they are pending in the mempool.

As a result, transactions may be executed much later than intended by the user. Consequently, users or the protocol might receive unfavorable prices, as validators could delay transaction inclusion to manipulate trade outcomes.

function swapERC20ToStAvail(address allowanceTarget, bytes calldata data) external {
    (IERC20 tokenIn, IERC20 tokenOut, uint256 inAmount, uint256 minOutAmount,) =
                abi.decode(data[4:], (IERC20, IERC20, uint256, uint256, Transformation[]));
    if (address(tokenOut) != address(avail)) revert InvalidOutputToken();
    tokenIn.safeTransferFrom(msg.sender, address(this), inAmount);
    tokenIn.forceApprove(allowanceTarget, inAmount);
    // slither-disable-next-line low-level-calls
    (bool success, bytes memory result) = swapRouter.call(data);
    if (!success) revert SwapFailed(string(result));
    uint256 outAmount = abi.decode(result, (uint256));
    if (outAmount < minOutAmount) revert ExceedsSlippage();
    // slither-disable-next-line unused-return
    avail.approve(address(stAvail), outAmount);
    stAvail.mintTo(msg.sender, outAmount);
}

function swapERC20ToStAvailWithPermit(
    address allowanceTarget,
    bytes calldata data,
    uint256 deadline,
    uint8 v,
    bytes32 r,
    bytes32 s
) external {
    (IERC20 tokenIn, IERC20 tokenOut, uint256 inAmount, uint256 minOutAmount,) =
                abi.decode(data[4:], (IERC20, IERC20, uint256, uint256, Transformation[]));
    if (address(tokenOut) != address(avail)) revert InvalidOutputToken();
    // if permit fails, assume executed
    try IERC20Permit(address(tokenIn)).permit(msg.sender, address(this), inAmount, deadline, v, r, s) {} catch {}
    tokenIn.safeTransferFrom(msg.sender, address(this), inAmount);
    tokenIn.forceApprove(allowanceTarget, inAmount);
    // slither-disable-next-line low-level-calls
    (bool success, bytes memory result) = swapRouter.call(data);
    if (!success) revert SwapFailed(string(result));
    uint256 outAmount = abi.decode(result, (uint256));
    if (outAmount < minOutAmount) revert ExceedsSlippage();
    // slither-disable-next-line unused-return
    avail.approve(address(stAvail), outAmount);
    stAvail.mintTo(msg.sender, outAmount);
}

function swapETHtoStAvail(bytes calldata data) external payable {
    (address tokenIn, IERC20 tokenOut, uint256 inAmount, uint256 minOutAmount,) =
                abi.decode(data[4:], (address, IERC20, uint256, uint256, Transformation[]));
    if (address(tokenIn) != 0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE) revert InvalidInputToken();
    if (address(tokenOut) != address(avail)) revert InvalidOutputToken();
    if (msg.value != inAmount) revert InvalidInputAmount();
    // slither-disable-next-line low-level-calls
    (bool success, bytes memory result) = swapRouter.call{value: msg.value}(data);
    if (!success) revert SwapFailed(string(result));
    uint256 outAmount = abi.decode(result, (uint256));
    if (outAmount < minOutAmount) revert ExceedsSlippage();
    // slither-disable-next-line unused-return
    avail.approve(address(stAvail), outAmount);
    stAvail.mintTo(msg.sender, outAmount);
}

Allow users to set a fixed deadline as a parameter and never automatically assign the deadline as block.timestamp within the function, as that would effectively mean that the transaction has no deadline.

All project, especially the AvailDepository.sol and DeqRouter.sol contracts currently lack event emissions, which are essential for providing transparency and monitoring capabilities for their respective functionalities.

In the AvailDepository.sol contract, events serve as crucial notifications for deposit and update actions. They enable users and external systems to track deposits of Avail ERC20 tokens to the depository on Avail and updates to the depository address. Without events, stakeholders may lack visibility into these critical contract interactions.

Similarly, in the DeqRouter.sol contract, events are necessary for transparent notification of token swap transactions to staked Avail. Events would allow users and external systems to monitor contract activities, ensuring that swaps are executed correctly and under user expectations.

function deposit() external onlyRole(DEPOSITOR_ROLE) {
    uint256 amount = avail.balanceOf(address(this));
     // keep 1 wei so slot stays warm, intentionally leave return unused, since OZ impl does not return false
    // slither-disable-next-line unused-return
    avail.approve(address(bridge), amount - 1);
    bridge.sendAVAIL(depository, amount - 1);
}

We recommend adding appropriate event emissions to the project.

The catch blocks of the mintWithPermit() and swapERC20ToStAvailWithPermit() functions could benefit from including error-handling code to address potential front-running attack scenarios. Specifically, the code mentioned should verify if the allowance amount is sufficient.

/// @notice Mints LST based on the amount of Avail staked with permit
/// @dev Reverts if amount is 0
/// @param amount Amount of Avail to stake
/// @param deadline Deadline for the permit
/// @param v Signature v
/// @param r Signature r
/// @param s Signature s
function mintWithPermit(uint256 amount, uint256 deadline, uint8 v, bytes32 r, bytes32 s) external {
    if (amount == 0) revert ZeroAmount();
    uint256 shares = previewMint(amount);
    // slither-disable-next-line events-maths
    assets += amount;
    _mint(msg.sender, shares);
        try IERC20Permit(address(avail)).permit(msg.sender, address(this), amount, deadline, v, r, s) {} catch {}
    avail.safeTransferFrom(msg.sender, depository, amount);
}

/// @notice Swaps an ERC20 token to staked Avail with permit
/// @param allowanceTarget Address of the allowance target from 0x API
/// @param data Data for the swap from 0x API
function swapERC20ToStAvailWithPermit(
    address allowanceTarget,
    bytes calldata data,
    uint256 deadline,
    uint8 v,
    bytes32 r,
    bytes32 s
) external whenNotPaused {
    (IERC20 tokenIn, IERC20 tokenOut, uint256 inAmount, uint256 minOutAmount,) =
                abi.decode(data[4:], (IERC20, IERC20, uint256, uint256, Transformation[]));
    if (address(tokenOut) != address(avail)) revert InvalidOutputToken();
    // if permit fails, assume executed
    try IERC20Permit(address(tokenIn)).permit(msg.sender, address(this), inAmount, deadline, v, r, s) {} catch {}
    tokenIn.safeTransferFrom(msg.sender, address(this), inAmount);
    tokenIn.forceApprove(allowanceTarget, inAmount);
    // slither-disable-next-line low-level-calls
    (bool success, bytes memory result) = swapRouter.call(data);
    if (!success) revert SwapFailed(string(result));
    uint256 outAmount = abi.decode(result, (uint256));
    if (outAmount < minOutAmount) revert ExceedsSlippage();
    // slither-disable-next-line unused-return
    avail.approve(address(stAvail), outAmount);
    stAvail.mintTo(msg.sender, outAmount);
}

It is recommended to add a check for verifying the allowance amount after the permit function call inside the catch block.

Commentary from the client:

The error handling wastes gas without any added usefulness. Any added allowance checks would already happen in the consequent transfers.

The DeqRouter.sol contract facilitates the swapping of ERC20 tokens to staked Avail tokens. However, it does not incorporate a mechanism to handle fee-on-transfer tokens effectively. Fee-on-transfer tokens are ERC20 tokens that implement a fee mechanism on transfers, deducting a percentage of tokens from each transfer and redistributing them to holders or a designated address.

When swapping fee-on-transfer tokens using the DeqRouter.sol contract, users may experience discrepancies between the requested transfer amount and the actual amount received due to the deduction of fees during token transfers.However, the 0x protocol does not fully support fees-on-transfer tokens. Transactions involving such tokens are likely to revert due to slippage protection.

However, the 0x protocol does not fully support fees-on-transfer tokens. Transactions involving such tokens are likely to revert due to slippage protection (Exceptional ERC20s — 0x Protocol 4.1 documentation).

function swapERC20ToStAvail(address allowanceTarget, bytes calldata data) external {
    (IERC20 tokenIn, IERC20 tokenOut, uint256 inAmount, uint256 minOutAmount,) =
                abi.decode(data[4:], (IERC20, IERC20, uint256, uint256, Transformation[]));
    if (address(tokenOut) != address(avail)) revert InvalidOutputToken();
    tokenIn.safeTransferFrom(msg.sender, address(this), inAmount);
    tokenIn.forceApprove(allowanceTarget, inAmount);
    // slither-disable-next-line low-level-calls
    (bool success, bytes memory result) = swapRouter.call(data);
    if (!success) revert SwapFailed(string(result));
    uint256 outAmount = abi.decode(result, (uint256));
    if (outAmount < minOutAmount) revert ExceedsSlippage();
    // slither-disable-next-line unused-return
    avail.approve(address(stAvail), outAmount);
    stAvail.mintTo(msg.sender, outAmount);
}

We recommend explicitly documenting the protocol's stance regarding fees-on-transfer tokens.

Commentary from the client:

There is no support for fee-on-transfer tokens, this has been updated in the contract NatSpec documentation.

The comment states that the previewFulfill() function reverts if till is less than or equal to lastFulfillment, implying that the function may revert even if till equals lastFulfillment. However, in the current implementation, the function does not revert when till is equal to lastFulfillment. Instead, it returns a value of 0, which is inconsistent with the provided comment.

 /// @notice Returns fulfilment amount between lastFulfillment and till
/// @dev Reverts if till is less than or equal to lastFulfillment
/// @param till Token ID to iterate till
function previewFulfill(uint256 till) public view returns (uint256 amount) {
                        return      withdrawals[till].accAmount    -       withdrawals[lastFulfillment].accAmount;
}

Update the comment for the previewFulfill() function to reflect the actual behavior.

The swapERC20ToStAvail(), swapERC20ToStAvailWithPermit(), and swapETHtoStAvail() functions of DeqRouter don't have any checks of the selector in the data argument. It allows users to use any function that has a compatible abi.

For example, a call to some public constant on the 0x side will be successful because the automatically created getters for public variables don't require any arguments, so they'll just skip the part of data that contains tokenIn, tokenOut, inAmount, minOutAmount, and transformations and return some constant value that will be approved to stAvail.

It's only an issue if there's AVAIL token on the balance of DeqRouter, which is not planned.

The users can also set allowanceTarget to their address in this case to get their transferred tokens back because 0x will not use any allowance.

/// @notice Swaps an ERC20 token to staked Avail
/// @param allowanceTarget Address of the allowance target from 0x API
/// @param data Data for the swap from 0x API
function swapERC20ToStAvail(address allowanceTarget, bytes calldata data) external {
    (IERC20 tokenIn, IERC20 tokenOut, uint256 inAmount, uint256 minOutAmount,) =
                abi.decode(data[4:], (IERC20, IERC20, uint256, uint256, Transformation[]));
    if (address(tokenOut) != address(avail)) revert InvalidOutputToken();
    tokenIn.safeTransferFrom(msg.sender, address(this), inAmount);
    tokenIn.forceApprove(allowanceTarget, inAmount);
    // slither-disable-next-line low-level-calls
    (bool success, bytes memory result) = swapRouter.call(data);
    if (!success) revert SwapFailed(string(result));
    uint256 outAmount = abi.decode(result, (uint256));
    if (outAmount < minOutAmount) revert ExceedsSlippage();
    // slither-disable-next-line unused-return
    avail.approve(address(stAvail), outAmount);
    stAvail.mintTo(msg.sender, outAmount);
}
 
/// @notice Swaps an ERC20 token to staked Avail with permit
/// @param allowanceTarget Address of the allowance target from 0x API
/// @param data Data for the swap from 0x API
function swapERC20ToStAvailWithPermit(
    address allowanceTarget,
    bytes calldata data,
    uint256 deadline,
    uint8 v,
    bytes32 r,
    bytes32 s
) external {
    (IERC20 tokenIn, IERC20 tokenOut, uint256 inAmount, uint256 minOutAmount,) =
                abi.decode(data[4:], (IERC20, IERC20, uint256, uint256, Transformation[]));
    if (address(tokenOut) != address(avail)) revert InvalidOutputToken();
    // if permit fails, assume executed
        try IERC20Permit(address(tokenIn)).permit(msg.sender, address(this), inAmount, deadline, v, r, s) {} catch {}
    tokenIn.safeTransferFrom(msg.sender, address(this), inAmount);
    tokenIn.forceApprove(allowanceTarget, inAmount);
    // slither-disable-next-line low-level-calls
    (bool success, bytes memory result) = swapRouter.call(data);
    if (!success) revert SwapFailed(string(result));
    uint256 outAmount = abi.decode(result, (uint256));
    if (outAmount < minOutAmount) revert ExceedsSlippage();
    // slither-disable-next-line unused-return
    avail.approve(address(stAvail), outAmount);
    stAvail.mintTo(msg.sender, outAmount);
}
 
/// @notice Swaps ETH to staked Avail
/// @param data Data for the swap from 0x API
function swapETHtoStAvail(bytes calldata data) external payable {
    (address tokenIn, IERC20 tokenOut, uint256 inAmount, uint256 minOutAmount,) =
                abi.decode(data[4:], (address, IERC20, uint256, uint256, Transformation[]));
      if (address(tokenIn) != 0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE) revert InvalidInputToken();
    if (address(tokenOut) != address(avail)) revert InvalidOutputToken();
    if (msg.value != inAmount) revert InvalidInputAmount();
    // slither-disable-next-line low-level-calls
     (bool success, bytes memory result) = swapRouter.call{value: msg.value}(data);
    if (!success) revert SwapFailed(string(result));
    uint256 outAmount = abi.decode(result, (uint256));
    if (outAmount < minOutAmount) revert ExceedsSlippage();
    // slither-disable-next-line unused-return
    avail.approve(address(stAvail), outAmount);
    stAvail.mintTo(msg.sender, outAmount);
}

Decode the selector and check that it is equal to 0x415565b0 (transformERC20()).

Commentary from the client:

The selector would differ based on the type of swap call and can change in the future due to changes in the 0x proxy contract. Hence, we are leaving this as-is.

AvailDepository, StakedAvail, and AvailWithdrawalHelper have initializer functions open on the implementation contracts which can cause unnecessary confusion in case some user calls them to set storage variables or claim the ownership.

constructor(IERC20 newAvail, IAvailBridge newBridge) {
    if (address(newAvail) == address(0) || address(newBridge) == address(0)) revert ZeroAddress();
    avail = newAvail;
    bridge = newBridge;
}

constructor(IERC20 newAvail) {
    if (address(newAvail) == address(0)) revert ZeroAddress();
    avail = newAvail;
}

  constructor(IERC20 newAvail) {
      if (address(newAvail) == address(0)) revert ZeroAddress();
      avail = newAvail;
  }

Add _disableInitializers() to the constructors of the contracts.

The only way to withdraw the deposited AVAIL in AvailDepository is through the deposit() function. If there is an external issue with the bridge or the L2, the users will not be able to withdraw the funds.

function deposit() external whenNotPaused onlyRole(DEPOSITOR_ROLE) {
    uint256 amount = avail.balanceOf(address(this));
     // keep 1 wei so slot stays warm, intentionally leave return unused, since OZ impl does not return false
    // slither-disable-next-line unused-return
    avail.approve(address(bridge), amount - 1);
    bridge.sendAVAIL(depository, amount - 1);
}

We recommend adding an alternative function that will send the funds directly to the AvailWithdrawalHelper without involving the bridge.