Assessment reportsPublic findings
Back to Zellic site
↗
Assessment reports>Orderly Network>Threat Model>executeWithdrawAction
GeneralOverview
Findings
Critical (1)
High (4)
Medium (3)
Informational (1)
DiscussionAdditional checksOverall issues with some functionsThe `changeFeeCollector` does not revertThe `tokenHash` is not a hash from `tokenAddress`
Threat ModelWhat are threat models?AccountTypeHelper.solAccountTypePositionHelper.solCrossChainRelayUpgradeable.solFeeManager.sol
Ledger.solaccountDepositaccountWithDrawFinishexecuteAdlexecuteLiquidationexecuteProcessValidatedFuturesexecuteSettlementexecuteWithdrawActionsetCrossChainManagersetFeeManagersetMarketManagersetOperatorManagerAddresssetVaultManager
LedgerComponent.solLedgerCrossChainManagerUpgradeable.solMarketManager.solOperatorManager.solOperatorManagerComponent.solSignature.solVault.solVaultCrossChainManagerUpgradeable.solVaultManager.sol
Audit ResultsSummary

Function: executeWithdrawAction(EventTypes.WithdrawData withdraw, uint64 eventId)

This allows to withdraw previously deposited funds. The vault in the destination chain should contain enough liquidity to withdraw from.

struct WithdrawData { uint128 tokenAmount; - The `accountId` should have enough local balance of tokens that will be withdrawn. uint128 fee; - The withdraw fee. It will be transferred to the `feeCollectorAccount` after successful completion of withdrawal. uint256 chainId; - The destination chain ID from which the funds will be withdrawn. bytes32 accountId; - Should be related to the `sender` and `brokerId`. bytes32 r; - Verify signature. bytes32 s; - Verify signature. uint8 v; - Verify signature. address sender; - Address should be related to the `accountId`. uint64 withdrawNonce; - The unique number of withdraw action, should not be repeated. address receiver; - Address of receiver of funds. There is no verification. The address will be passed to the `chainId` to transfer funds. uint64 timestamp; - It is not used here. string brokerId; - Should be an allowed broker. string tokenSymbol; - Should be an allowed token. }

Inputs

  • withdraw

    • Control: Full control by operator.

    • Constraints: Balance of accountId is enough, the brokerId and token is allowed, and the balance of token in the destination chainId is enough. The withdrawNonce should not be less than the last nonce.

    • Impact: The funds of the tokens from accountId will be transferred to the receiver in chainId.

  • eventId

    • Control: Full control by operator.

    • Constraints: There are not any checks.

    • Impact: The unique ID of withdraw action.

Branches and code coverage

Intended branches

  • Check if the branch has test coverage.

  • Include function calls.

  • End sentences with periods.

Negative behavior

  • Sender is not related to the accountId.

  • Untrusted brokerId.

  • Untrusted tokenSymbol.

  • The withdrawNonce is less than the last.

  • tokenAmount is more than accountId balance.

  • tokenAmount is more than Vault balance in chainId.

  • receiver is the zero address.

  • The frozenBalance of accountId is not zero.

  • The frozenBalance of accountId is not zero.

Function call analysis

  • vaultManager.getAllowedBroker(brokerHash)

    • External/Internal? External.

    • Argument control? brokerHash is controlled by operator.

    • Impact: Return true if the brokerHash is allowed.

  • vaultManager.getAllowedChainToken(tokenHash, withdraw.chainId)

    • External/Internal? External.

    • Argument control? tokenHash is calculated; withdraw.chainId is controlled by operator.

    • Impact: Return true if the tokenHash in chainId is allowed.

  • vaultManager.getMaxWithdrawFee(tokenHash)

    • External/Internal? External.

    • Argument control? tokenHash is calculated.

    • Impact: Return the max fee value for tokenHash.

  • vaultManager.getBalance(tokenHash, withdraw.chainId)

    • External/Internal? External.

    • Argument control? tokenHash is calculated; withdraw.chainId is controlled by operator.

    • Impact: Return the balance of all deposited funds for tokenHash in chainId.

  • Signature.verifyWithdraw(withdraw.sender, withdraw)

    • External/Internal? Internal.

    • Argument control? All arguments are controlled by operator.

    • Impact: Verify that sender has signed the withdraw data.

  • account.frozenBalance(withdraw.withdrawNonce, tokenHash, withdraw.tokenAmount)

    • External/Internal? Internal.

    • Argument control? All arguments are controlled by operator.

    • Impact: Decrease the balance of accountId and increase the frozen balance.

  • vaultManager.frozenBalance(tokenHash, withdraw.chainId, withdraw.tokenAmount - withdraw.fee)

    • External/Internal? External.

    • Argument control? All arguments are controlled by operator.

    • Impact: Increase the frozen balance of tokenHash in chainId.

  • ILedgerCrossChainManager(crossChainManagerAddress).withdraw(withdraw)

    • External/Internal? External.

    • Argument control? withdraw is controlled by operator.

    • Impact: Send cross-chain LayerZero message to perform withdraw action in Vault in destination chain.

Zellic © 2024Back to top ↑