joestakey - Incorrect adjusted amount calculation in `getAdjustedLongAndShortTokenAmounts()` always reverts
Closed this issue · 0 comments
joestakey
medium
Incorrect adjusted amount calculation in getAdjustedLongAndShortTokenAmounts()
always reverts
Summary
The cases are handled incorrectly, leading to the call always reverting, which means markets where the short and long token are the same will never work.
Vulnerability Detail
The function calculates the adjusted token amounts that would minimize the price impact.
It first handles the case poolLongTokenAmount < poolShortTokenAmount
.
The issue is that it performs the incorrect subtraction poolLongTokenAmount - poolShortTokenAmount
, which will underflow.
File: contracts/deposit/ExecuteDepositUtils.sol
392: if (poolLongTokenAmount < poolShortTokenAmount) {
393: uint256 diff = poolLongTokenAmount - poolShortTokenAmount;//@audit underflow
The same problem happens in the other block, which will always underflow (unless both amounts are 0, which is not a relevant case in a deposit context)
File: contracts/deposit/ExecuteDepositUtils.sol
401: } else {
402: uint256 diff = poolShortTokenAmount - poolLongTokenAmount;
Impact
getAdjustedLongAndShortTokenAmounts()
always reverts.
This function is called internally in executeDeposit
when market.longToken == market.shortToken
.
This means deposits do not work in this case: markets where the long and short token are the same will simply not work.
Code Snippet
Tool used
Manual Review
Recommendation
File: contracts/deposit/ExecuteDepositUtils.sol
392: if (poolLongTokenAmount < poolShortTokenAmount) {
-393: uint256 diff = poolLongTokenAmount - poolShortTokenAmount;
+393: uint256 diff = poolShortTokenAmount - poolLongTokenAmount;
File: contracts/deposit/ExecuteDepositUtils.sol
401: } else {
-402: uint256 diff = poolShortTokenAmount - poolLongTokenAmount;
+402: uint256 diff = poolLongTokenAmount - poolShortTokenAmount;
Duplicate of #165