OneDex Audit

The quorum serves as the benchmark for validating a transaction that is set to be executed. Also it will be used to validate if the number of relayers is larger or equal than this value: ```rust=28 require!( usize::try_from(quorum) .map(|initial_quorum| relayers.len() >= initial_quorum) .unwrap_or(false), errors::QuorumError::BoardTooSmall ); ``` However, when updating the Quorum or Relayer, such validation is missing; as a result, the quorum can be set much larger than the number of relayers or the number of relayers will not meet the quorum. Either case will cause the denial of service on `execute_transfer`.

When adding a relayer, checks are made against the relayer's solana address to ensure it is non-zero and unique. ```rust=93 require!( relayer.solana_address != Pubkey::default(), RelayerError::AddressNotValid ); require!( !self.is_relayer(&relayer.solana_address), RelayerError::AlreadyRelayer ); ``` However, there are no checks made against the relayer's Secp256k1 public key. This allows the possibility of adding a relayer with a zero Secp256k1 public key or a public key matching another relayer's, possibly causing issues when trying to reach quorum.

According to code logic of the `whitelist_token()`, `set_token_min_limit()`, and `set_token_max_limit()` functions, we notice that there is no validation in place for the values of `min_amount` and `max_amount` within the `WhitelistedToken` stats account. Consequently, this situation may lead to disruptions in the cross-chain transfer-out service. **whitelisted_token.rs** ```rust=16 pub fn whitelist_token(&mut self, min_amount: u64, max_amount: u64) -> Result<()> { self.is_whitelisted = true; self.min_amount = min_amount; self.max_amount = max_amount; Ok(()) } ``` ```rust=28 pub fn set_token_min_limit(&mut self, amount: u64) -> Result<()> { self.min_amount = amount; Ok(()) } pub fn set_token_max_limit(&mut self, amount: u64) -> Result<()> { self.max_amount = amount; Ok(()) } ``` Without comparing the two arguments, `min_amount` and `max_amount`, it's possible that the value of `self.min_amount` could exceed the value of `self.max_amount`. **whitelisted_token.rs** ```rust=38 pub fn deposit(&mut self, amount: u64) -> Result<()> { require!( amount >= self.min_amount, WhitelistedTokenError::DepositTooLow ); require!( amount <= self.max_amount, WhitelistedTokenError::DepositTooHigh ); self.increase_balance(amount) } ``` When the two `require!()` statements, located at L39 and L43, are executed, only one of them can succeed because the `amount` cannot simultaneously be greater than `self.min_amount` and less than `self.max_amount`. As a result, the `deposit()` function always fails to execute. For instance, with `self.min_amount = 50`, `self.max_amount = 10`, and `amount = 60`, only one of the conditions can be met.

The `recipient` field of a `Deposit` account is 32 bytes long. ```rust=10 pub recipient: [u8; 32], ``` The bridge is assumed to be between Solana and Ethereum due to the type hash used for relayer signatures, but addresses on Ethereum are 20 bytes long instead of 32 bytes. The auditing team would like to double-check if deposit recipients should actually be of type `[u8; 20]` instead of type `[u8; 32]`.

Based on the context in `token-safe`, there are two extra payments for users related to the new account's rent for the protocol: - The ATA account's rent for `safe_settings` while depositing, if there is no `init_supply` before the user's deposit. - The next Batch account's rent when the new Batch account needs to be initialized. Since the creation of a new account may result in different charges for users' deposits, we would like to check with the team if this is the intended design.

The `relayer` argument is accessed in the `AddRelayer` and `RemoveRelayer` instructions, but never used. ```rust #[derive(Accounts)] #[instruction(relayer: Pubkey)] pub struct AddRelayer<'info> { ... } ```