# SiloRouter - Audit Report

## Overview

This report presents the findings from the audit of the `SiloRouter` and `SiloRouterImplementation` contracts. These contracts act as UX utilities, allowing users to batch multiple actions such as deposits, withdrawals, borrowing, and repayments in a single transaction and interact with the rest of Silo Protocol V2 protocol.

## Findings

### 1. Unrestricted ETH receival.
The `SiloRouter::receive()` function allows the contract to accept native tokens unconditionally, however, it is only intended for unwrapping native tokens. For this reason, consider implementing an `onlyNativeToken` modifier to restrict direct ETH transfers and ensure that users can only send ETH via the `SiloRouter::multicall()` function, reducing potential issues. In general, it is better to have a single entry point for ETH into the Router, preventing potential user experience issues where users mistakenly send ETH to the contract without triggering the `SiloRouter::multicall` function.

/// @dev Needed for unwrapping native tokens
receive() external whenNotPaused payable {
    // `multicall` method may call `IWrappedNativeToken.withdraw()`
    // and we need to receive the withdrawn native token unconditionally
}

### 2. Immutable `implementation` address.
The `IMPLEMENTATION` variable is declared as `immutable`, meaning it cannot be modified after deployment. But, if there is ever a need to change the implementation contract (either for removing a functionality or extending the previous implementation), it would require a new contract deployment instead of simply updating the address. Consider replacing immutable with a `setImplementation` function restricted by `onlyOwner`, allowing for controlled upgrades without requiring a full redeployment and, eventually, bigger flexibility.

/// @notice The address of the implementation contract
address public immutable IMPLEMENTATION;

### 3. Issues on `repay` flow documentation comments.
The comments in `SiloRouterImplementation.sol` describing the repay flow contain some inaccuracies. The `full repay token using SiloRouter.multicall` scenario incorrectly calls `sendValueAll`, which is unnecessary in this flow. Additionally, the `repayAllNative` scenario is missing from the documented flows. Consider updating the documentation to include the `repayAllNative` flow and, also, correct the `repayAll` scenario removing the unnecessary `sendValueAll` call.

/**
- full repay token using SiloRouter.multicall
    SiloRouter.repayAll(ISilo _silo, address _borrower)
    SiloRouter.sendValueAll(address payable _to)
*/    

### 4. Missing validation for `IWrappedNativeToken` in `SiloRouterImplementation::unwrap` and `SiloRouterImplementation::wrap`.
`SiloRouterImplementation` does not validate whether the `IWrappedNativeToken` instance provided is the actual wrapped native token of this chain. Currently, it only takes an interface as input without verifying if this interfaces, indeed, includes the actual wrapped native token address. In this way, the router will be capable of calling `withdraw` and `deposit` function of other smart contracts as well. Consider adding a check to ensure that the provided contract is indeed the expected wrapped native token to prevent potential misuse or unintended behavior.

/// @inheritdoc ISiloRouterImplementation
function wrap(IWrappedNativeToken _native, uint256 _amount) public payable virtual {
    _native.deposit{value: _amount}();
}

/// @inheritdoc ISiloRouterImplementation
function unwrap(IWrappedNativeToken _native, uint256 _amount) public payable virtual {
    _native.withdraw(_amount);
}

### 5. Inconsistent parameter naming for Wrapped Native Token in `SiloRouterImplementation::unwrap` and `SiloRouterImplementation::wrap`.
The parameter used for the wrapped native token in these functions is named `native`, which may cause confusion, as it suggests it refers to the actual native token rather than the wrapped version (since there is a common practice to set `native token == 0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE` some times). Consider renaming the parameter to `wrappedNative` to enhance clarity.

/// @inheritdoc ISiloRouterImplementation
function wrap(IWrappedNativeToken _native, uint256 _amount) public payable virtual {
    _native.deposit{value: _amount}();
}

/// @inheritdoc ISiloRouterImplementation
function unwrap(IWrappedNativeToken _native, uint256 _amount) public payable virtual {
    _native.withdraw(_amount);
}

### 6. Handling `sendValueAll` for zero ETH balance cases.
Currently, `SiloRouterImplementation::sendValueAll()` only executes when the balance is nonzero, which means fallback functions in receiving contracts will not be triggered in some cases. Consider modifying `sendValue` to execute even when the balance is zero to ensure that expected fallback functionality (if any) is **always** invoked.

function sendValueAll(address payable _to) external payable virtual {
    uint256 balance = address(this).balance;

    if (balance != 0) { // expect this fn to be used as a sanity check at the end of the multicall
        sendValue(_to, balance);
    }
}

### 7. About gas forwarding in `sendValue`.
The `sendValue` function currently forwards all available gas to the recipient, as it is, under the hood, calling `to.call{value: amount}("")`. This approach may have some risks, as the recipient contract can consume excessive gas, leading to the transaction to revert. Effectively, it gives to `to` address the full access to revert OOG the `multicall` bundle. Consider implementing a gas limit when forwarding ETH to mitigate this risk and also have bigger flexibility to update it accordignly.

/// @inheritdoc ISiloRouterImplementation
function sendValue(address payable _to, uint256 _amount) public payable virtual {
    Address.sendValue(_to, _amount);
}

## Conclusion

All in all, the audit identified some areas of potential concern regarding contract functionality of `SiloRouter.sol` and `SiloRouterImplementation.sol`, but no serious security vulnerabilities appear to be in them. Addressing these issues would improve the robustness of `SiloRouter` and the implementation, enhancing its overall usability.