Closed code423n4 closed 1 year ago
https://github.com/code-423n4/2023-08-goodentry/blob/71c0c0eca8af957202ccdbf5ce2f2a514ffe2e24/contracts/PositionManager/OptionsPositionManager.sol#L493 https://github.com/code-423n4/2023-08-goodentry/blob/71c0c0eca8af957202ccdbf5ce2f2a514ffe2e24/contracts/PositionManager/OptionsPositionManager.sol#L473
The protocol implements Router01 from UniswapV2 to do any swaps of tokens in OptionsPositionManager.sol, but Router01 is deprecated and bugged and should not be used anymore.
OptionsPositionManager.sol
As can be seen in the UniswapV2 documentation https://docs.uniswap.org/contracts/v2/reference/smart-contracts/router-01 Router01 should not be used anymore because of the discovery of a bug in the function getAmountIn, function which is used in the swapExactTokensForTokens, and as you can see, it is even used in the OptionsPositionManager.sol to calculate amountsIn https://github.com/code-423n4/2023-08-goodentry/blob/71c0c0eca8af957202ccdbf5ce2f2a514ffe2e24/contracts/PositionManager/OptionsPositionManager.sol#L493 and it is stated to not be used https://docs.uniswap.org/contracts/v2/reference/smart-contracts/router-01#getamountin. The recommendation from Uniswap is to use Router02, since it doesn't have the same problems.
getAmountIn
swapExactTokensForTokens
amountsIn
Manual review
Implement the UniswapV2 Router02 instead of Router01, since Router01 has known bugs and it is deprecated.
Uniswap
141345 marked the issue as primary issue
This is an interface not code. Naming doesnt matter
Keref marked the issue as sponsor disputed
gzeon-c4 marked the issue as unsatisfactory: Invalid
Lines of code
https://github.com/code-423n4/2023-08-goodentry/blob/71c0c0eca8af957202ccdbf5ce2f2a514ffe2e24/contracts/PositionManager/OptionsPositionManager.sol#L493 https://github.com/code-423n4/2023-08-goodentry/blob/71c0c0eca8af957202ccdbf5ce2f2a514ffe2e24/contracts/PositionManager/OptionsPositionManager.sol#L473
Vulnerability details
Impact
The protocol implements Router01 from UniswapV2 to do any swaps of tokens in
OptionsPositionManager.sol
, but Router01 is deprecated and bugged and should not be used anymore.Proof of Concept
As can be seen in the UniswapV2 documentation https://docs.uniswap.org/contracts/v2/reference/smart-contracts/router-01 Router01 should not be used anymore because of the discovery of a bug in the function
getAmountIn
, function which is used in theswapExactTokensForTokens
, and as you can see, it is even used in theOptionsPositionManager.sol
to calculateamountsIn
https://github.com/code-423n4/2023-08-goodentry/blob/71c0c0eca8af957202ccdbf5ce2f2a514ffe2e24/contracts/PositionManager/OptionsPositionManager.sol#L493 and it is stated to not be used https://docs.uniswap.org/contracts/v2/reference/smart-contracts/router-01#getamountin. The recommendation from Uniswap is to use Router02, since it doesn't have the same problems.Tools Used
Manual review
Recommended Mitigation Steps
Implement the UniswapV2 Router02 instead of Router01, since Router01 has known bugs and it is deprecated.
Assessed type
Uniswap