Check vaults due to router allowance

[0xdcota]

C-3 Using the user’s router allowance, anyone can steal user’s funds.

Description

A vault is a user input for all actions of the router bundle.

Suppose Alice gives the router an allowance to spend 1000 tokens and intends to deposit these tokens into vault X. An attacker who sees Alice's approval in the mempool can front-run Alice's deposit with their own bundle.

To pass the checks of _safePullTokenFrom, they pass sender and receiver both as Alice only. However, please note that the vault is user-input, so anyone can pass anything there and make you believe Alice is the receiver when she is not. This way, anyone can use anyone's allowance and run away with their funds.

// DEPOSIT
// sender = receiver = user
(IVault vault, uint256 amount, address receiver, address sender) =
          abi.decode(args[i], (IVault, uint256, address, address));
----
_safePullTokenFrom(token, sender, receiver, amount);
----
vault.deposit(amount, receiver);

----------------------------
function _safePullTokenFrom(
    address token,
    address sender,
    address owner,
    uint256 amount
)
internal
{
    if (sender != address(this) && (sender == owner || sender == msg.sender)) {
      ERC20(token).safeTransferFrom(sender, address(this), amount);
    }
}

Remediation to consider

Consider whitelisting vaults and verifying that the sender is equal to msg.sender inside _safePullTokenFrom. Implementing both changes would reduce the possible exposure to the contract due to user input and ensure that the sender is the actual user performing the transaction