The Standard

Summary

The SmartVaultV3::swap function is designed to facilitate token exchanges using an external swap router. The function parameters include a deadline set to block.timestamp and a sqrtPriceLimitX96 set to 0. These parameters raise concerns regarding the potential for failed transactions and unbounded slippage, which could negatively impact the reliability of the contract and the user's funds.

Vulnerability Details

The SmartVaultV3::swap function has parameters deadline and sqrtPriceLimitX96 respectively set to block.timestamp and 0:

The deadline parameter is set to the current block timestamp. This means the transaction must be executed immediately, leaving no time buffer for the transaction to be mined. In a congested network, this could lead to transaction failures, as miners may not include the transaction in the current block.

The sqrtPriceLimitX96 parameter is set to 0, indicating no limit on the execution price for the swap. This exposes the user to high slippage in volatile market conditions, potentially resulting in a swap execution at an unfavorable rate.

Impact

Setting the sqrtPriceLimitX96 parameter to 0 in SmartVaultV3::swap function means that there is no limit on the price for the swap. The swap will execute at the best available price between the specified input and output tokens, regardless of how the price may have moved. This could be risky in volatile markets, as the swap could execute at an unfavorable rate if the market price moves significantly between the time the transaction is sent and when it is mined.

Setting the sqrtPriceLimitX96 parameter to 0, makes the parameter useless. In production this parameter should not be 0. In the Uniswap V3 documentation for the example in the docs is said:

"sqrtPriceLimitX96: We set this to zero - which makes this parameter inactive. In production, this value can be used to set the limit for the price the swap will push the pool to, which can help protect against price impact or for setting up logic in a variety of price-relevant mechanisms."

Link to the documentation: https://docs.uniswap.org/contracts/v3/guides/swaps/single-swaps

Also, the impact of the setting deadline parameter to block.timestamp is that in some cases, transactions might be delayed due to network congestion or other reasons, causing them to miss their deadline. This could lead to the transaction failing unnecessarily.

Tools Used

Manual Review

Recommendations

Calculate an appropriate value for the sqrtPriceLimitX96 parameter based on the acceptable slippage percentage and set it accordingly to provide slippage protection.

Also, it is recommended to set the deadline parameter to a future timestamp, providing a time window during which the transaction can be mined. This window should be long enough to account for network congestion and potential delays but not too long as to expose the transaction to unnecessary market risk. A common practice is to add a buffer of several minutes (e.g., 15 minutes) to the current block timestamp: