Sonic Gateway security record

If `UpdateManager.update()` were to be invoked with both a list of new validators, and a new update verifier contract, the bridge can potentially become inoperative. This is due to the order in which the conditionals are executed inside this function. The first condition invokes `updateVerifier.setValidators()` if there is a list of `newValidators` specified during the call. However, the third condition, will update `updateVerifier = IUpdateVerifier(_updateVerifier)` in the case where there is a non-zero `_updateVerifier` address specified during the call. With the reassignment of the `updateVerifier` address occuring after `newValidators` are updated in the previous `updateVerifier` address, this new `updateVerifier` address does not have the proper set of new validators assigned in its state. Further, assuming that the `IUpdateVerifier(ValidatorsRegistry)` contract was deployed with the `UpdateManager` contract assigned as the owner, the bridge would be totally non-operational because there would be no way to invoke `setValidators()` on this new `updateVerifier` contract, because there are no validators defined in its state.

The `DirectExitAdministrator` contract validates the user’s proof of minted balance on Sonic and tracks users who have withdrawn their funds from the bridge. In contrast, the `ExitAdministrator` neither performs these validations nor tracks withdrawals. It merely calls the `TokenDeposit` contract, which trusts the `ExitAdministrator` without further verification. If the admin of the `ExitAdministrator` is compromised, the `IMPLEMENTATION_ROLE` could be granted to another address, potentially allowing them to steal all the funds from the bridge. Furthermore, the additional modularity provided by the `ExitAdministrator` seems redundant. Since it does not offer any meaningful functionality or validation and directly interacts with the `TokenDeposit` contract, its responsibilities can be handled by the `DirectExitAdministrator` after verifying the user’s proof.

`TokenDeposit.exitAdministrator` is intended to be a privileged address that can invoke `withdrawWhileDead()`. During the fix review the following conditional was included to `TokenDeposit.getDeadStatus()`: ``` if (exitAdministrator == address(0)) { return false; // The dying mechanism is disabled if no exitAdministrator is set. } ``` While the comment included above appears that there is some intention behind having this function return false when there is no `exitAdministrator` set, doing so presents multiple scenarios for users to have their funds stuck in the bridge. In the scenario where the bridge is non-operational due to insufficient validators participating to meet quorum at a given time, there would be no way for the exit administrator to be assigned, as that process requires the quorum of validators to sign a message indicating the assignment. Further, this system has no requirement that the `exitAdministrator` be assigned on creation. In the case where this address is not assigned before the bridge becomes non-operational due to decreased validator participation, there will be no way to assign it and funds will be lost.

The `claim()` function calls `mint()` on the minted token address, which returns a boolean indicating whether the minting succeeded. However, the returned value isn't checked, which may lead to unexpected consequences in case the minting fails.

The `verifyUpdate()` function calls `ECDSA.recover(messageHash, signatures[i])` without first applying `MessageHashUtils.toEthSignedMessageHash()` to the `messageHash`, which is compliant with the [EIP-191](https://eips.ethereum.org/EIPS/eip-191) standard for signed data. Additionally, this method is recommended as the safer approach in the `ECDSA` file. ``` * IMPORTANT: `hash` _must_ be the result of a hash operation for the * verification to be secure: it is possible to craft signatures that * recover to arbitrary addresses for non-hashed data. A safe way to ensure * this is by receiving a hash of the original message (which may otherwise * be too long), and then calling {MessageHashUtils-toEthSignedMessageHash} on it. ```