code-423n4 / 2022-06-nested-findings

0 stars 1 forks source link

Wrong Deadline #65

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-06-nested/tree/main/contracts/operators/Beefy/lp/BeefyZapBiswapLPVaultOperator.sol

Vulnerability details

the deadline is the timestamp after which the transaction will revert. the goal of this field is that the caller can set a deadline for the transaction so the transaction will not succeed in any arbitrary time in the future, and after this deadline, they can resubmit the transaction: https://help.uniswap.org/en/articles/5455497-my-transaction-has-been-pending-for-a-long-time-what-can-i-do#:~:text=Wait%20%E2%80%94%20the%20Uniswap%20interface%20has,you%20can%20resubmit%20your%20transaction. The problem is that the transaction will always occur at block.timestamp so setting the deadline to be block.timestamp + x minutes won't have any effect (for any x>=0). Therefore the transaction can still run in an arbitrary time in the future. block.timestamp is confused with the time the transaction was submitted. Practically, the transaction doesn't have a deadline!

Code instances:

https://github.com/code-423n4/2022-06-nested/tree/main/contracts/operators/Beefy/lp/BeefyZapUniswapLPVaultOperator.sol: swapExactTokensForTokens( swapAmountIn, 1, path, address(this), block.timestamp ) https://github.com/code-423n4/2022-06-nested/tree/main/contracts/operators/Beefy/lp/BeefyZapBiswapLPVaultOperator.sol: swapExactTokensForTokens( swapAmountIn, 1, path, address(this), block.timestamp ) https://github.com/code-423n4/2022-06-nested/tree/main/contracts/operators/Beefy/lp/BeefyZapBiswapLPVaultOperator.sol: swapExactTokensForTokens(tokenAmountIn, 0, path, address(this), block.timestamp) https://github.com/code-423n4/2022-06-nested/tree/main/contracts/operators/Beefy/lp/BeefyZapUniswapLPVaultOperator.sol: swapExactTokensForTokens(tokenAmountIn, 0, path, address(this), block.timestamp)

maximebrugel commented 2 years ago

Good point, but it is rather a new feature, where the NestedFactory could take a timestamp parameter (and not for each swaps).

jack-the-pug commented 2 years ago

I see no issues here. Using block.timestamp is a common practice when doing swap in the smart contract.