Solv Protocol- Vault Guardian

The `FunctionAuthorization._checkSingleTx()` of the Solv Vault Guardian relies on a static mapping of function selectors before proceeding with ACL checks to validate transactions. This design does not accommodate the mutable design of upgradeable contracts, where function selectors might change after upgrades. Particularly for upgradeable contracts with payable fallback functions, this static mapping approach can lead to erroneous behavior, as the guardian may authorize or block transactions based on outdated information. Additionally, if an upgradeable contract were approved, the implementation of the approved function is subject to change.

The Solv Vault Guardian implements a mechanism to validate native token transfers, through the `_checkNativeTransfer()` function to validate transactions based on a list of approved receivers `nativeTokenReceiver`. This check is invoked only when the transaction's calldata is empty. However, this approach does not account for payable functions where the calldata is non empty, thereby bypassing the `_checkNativeTransfer()` validation when `msg.value` is sent alongside calldata. This design choice introduces a discrepancy in how native token transfers are validated, potentially allowing for native token disbursements without proper authorization checks when calldata is present.

The Solv protocol introduces guardian checks through the Authorization mechanism before transactions of the Safe are executed. These checks, depending on the protocol the Safe is interacting with, primarily focus on assessing tokens in, tokens out, and the recipients of swapped funds. The aim is to ensure transactions comply with the allowed types and adhere to the rules set by the Authorization contract. However, the framework does not effectively guard against slippage attacks. It allows Safe owners to set potentially exploitative transaction parameters, such as a zero minimum for tokens out during swaps, potentially bypassing the protective intent of the Authorization checks.

Currently, some of the ACL implementations that allow token whitelisting only allow them to be added via `_addTokenWhitelist()` during deployment. There is no way to modify this token whitelist following deployment. This could be an issue if one of the tokens becomes compromised or is no longer supported, or, if there are additional tokens that would want to be supported by Solv in the future.

Smart contracts will often have `owner` variables to designate the person with special privileges to make modifications to the smart contract. Although all of the functions below can be invoked by the Governor role, we would like to note that actually invoking the functions that are added to the guard still has to go through the multisig process, which significantly mitigates the privilege this Governor has. The Governor can invoke the following functions on the Solv Vault Guard defined in `SolvVaultGaurdianForSafe`: - `setGuardAllowed()` to toggle whether an address for the Guard of the Safe can be assigned or not. - `setNativeTokenTransferAllowed()` to toggle whether direct native token transfers can be invoked from the Safe or not. - `addNativeTokenReceiver()` and `removeNativeTokenReceiver()` to define the set of addresses can be transferred native token directly. - `addAuthorizations()` and `removeAuthorizations()` to define the set of Authorizations stored in the contract. - `setAuthorizationEnabled()` allowing the Governor to specify which of the Authorizations stored in the contract will be enabled and therefore enforced. - `transferGovernance()` to a new address. - `forbidGovernance()` preventing any Governor privileged calls to ever be called again. <br> For all Authorizations reviewed, the Governor can invoke the following functions: - `addContractFuncs()`, `removeContractFuncs()`, `addContractFuncSig()`, and `removeContractFuncSig()` to define a set of contracts and functions in those contracts that can be invoked by the Safe. - `setContractACL()` to define the address of the ACL that will be used to further verify transactions. - `transferGovernance()` to a new address. - `forbidGovernance()` preventing any Governor privileged calls to ever be called again. <br> In `SolvMasterFundAuthorizationACL`, the Governor can invoke the following functions: - `addPoolIdWhitelist()` and `removePoolIdWhitelist()` to define the set of Solv pool Ids that will be supported by the Guard. <br> In `ERC3525ApproveAuthorization` and `ERC20ApproveAuthorization` the Governor can invoke the following functions to change the state of the ACL: - `addTokenSpenders()` and `removeTokenSpenders()`to define the set of addresses that can be approved to spend tokens from the Safe. <br> In `ERC20TransferAuthorization` the Governor can invoke the following functions: - `addTokenReceivers()` and `removeTokenReceivers()` to define the set of which tokens can be transferred to which addresses from the Safe.