[Lines of code](https://github.com/code-423n4/2022-10-traderjoe/blob/79f25d48b907f9d0379dd803fc2abc9c5f57db93/src/LBRouter.sol#L891) # Vulnerability details ## Vulnerable detail Function LBRouter._swapSupportingFeeOnTransferTokens is a helper function to swap exact tokens supporting for a fee on transfer tokens. This function will check the pair of _token and _tokenNext is JoePair or LBPair using _binStep. * If _binStep == 0, it will be a JoePair otherwise it will be an LBPair. // url = https://github.com/code-423n4/2022-10-traderjoe/blob/79f25d48b907f9d0379dd803fc2abc9c5f57db93/src/LBRouter.sol#L887-L903 if (_binStep == 0) { (uint256 _reserve0, uint256 _reserve1, ) = IJoePair(_pair).getReserves(); if (_token < _tokenNext) { uint256 _balance = _token.balanceOf(_pair); uint256 _amountOut = (_reserve1 * (_balance - _reserve0) * 997) / (_balance * 1_000); IJoePair(_pair).swap(0, _amountOut, _recipient, ""); } else { uint256 _balance = _token.balanceOf(_pair); uint256 _amountOut = (_reserve0 * (_balance - _reserve1) * 997) / (_balance * 1_000); IJoePair(_pair).swap(_amountOut, 0, _recipient, ""); } } else { ILBPair(_pair).swap(_tokenNext == ILBPair(_pair).tokenY(), _recipient); } As we can see when _binStep == 0 and _token < _tokenNext (in another word we swap through JoePair and pair'stoken0 is _token and token1 is _tokenNext), it will 1. Get the reserve of pair (reserve0, reserve1) 2. Get the current _token's balance 3. Calculate the _amountOut by using the formula _amountOut = (_reserve1 * (_balance - _reserve0) * 997) / (_balance * 1_000) 4. Call IJoePair(_pair).swap(0, _amountOut, _recipient, "") But the formula to calculate the amountOut seem weird. Is it true? Let's do the math here to confirm whether it's true or not. **Input:** * _reserve0 (r0): reserve of token0 in pair * _reserve1 (r1): reserve of token1 in pair * _balance (b): balance of token0 after transfering _token to pair. **Output:** * amountOut': the amount of token1 we can get. **Generate Formula** Cause JoePair [takes 0.3%]() of amountIn as fee, we get * amountIn = b - r0 * amountDeductFee = 0.997 * amountIn * balanceDeductFee = r0 + amountDeductFee Following the [constant product formula](), we have r0 * r1 = balanceDeductFee * (r1-amountOut) <=> amountOut' = r1 - r0 * r1 / balanceDeductFee <=> amountOut' = (r1 * balanceDeductFee - r0 * r1) / balanceDeductFee <=> amountOut' = (r1 * (r0 + amountDeductFee) - r0 * r1) / balanceDeductFee <=> amountOut' = (r1 * amountDeductFee) / balanceDeductFee <=> amountOut' = r1 * 0.997 * (b - r0) / (r0 + 0.997 * amountIn) <=> amountOut' = (r1 * (b - r0) * 997) / (r0 * 1000 + 997 * amountIn) <=> amountOut' = (r1 * (b - r0) * 997) / (b * 997 + 3 * r0) <=> amountOut' = (_reserve1 * (_balance - _reserve0) * 997) / (_balance * 997 + 3 * _reserve0) As we can see amountOut' is different from amountOut, the denominator of amountOut is _balance * 1000 when the denominator of amountOut' is _balance * 997 + 3 * _reserve0 ## Impact As we can see above, amountOut < amountOut' (cause _balance * 1000 > _balance * 997 + 3 * _reserve0), so user will get a smaller amount out than expected. ## Proof of concept We can check function _swapSupportingFeeOnTransferTokens in JoeRouter02.sol following this link * # JoeRouter02.sol // https://github.com/traderjoe-xyz/joe-core/blob/e9a5757d87d60858eee2858afd2fb5a63498fc56/contracts/traderjoe/JoeRouter02.sol#L374-L375 function _swapSupportingFeeOnTransferTokens(address[] memory path, address _to) internal virtual { ... amountInput = IERC20Joe(input).balanceOf(address(pair)).sub(reserveInput); amountOutput = JoeLibrary.getAmountOut(amountInput, reserveInput, reserveOutput); ... } # JoeLibrary.sol // https://github.com/traderjoe-xyz/joe-core/blob/e9a5757d87d60858eee2858afd2fb5a63498fc56/contracts/traderjoe/libraries/JoeLibrary.sol#L63-L74 function getAmountOut( uint256 amountIn, uint256 reserveIn, uint256 reserveOut ) internal pure returns (uint256 amountOut) { require(amountIn > 0, "JoeLibrary: INSUFFICIENT_INPUT_AMOUNT"); require(reserveIn > 0 && reserveOut > 0, "JoeLibrary: INSUFFICIENT_LIQUIDITY"); uint256 amountInWithFee = amountIn.mul(997); uint256 numerator = amountInWithFee.mul(reserveOut); uint256 denominator = reserveIn.mul(1000).add(amountInWithFee); amountOut = numerator / denominator; } As we can see from code above, the formula to calculate amountOut is the same as the calculation I described in **Vulnerable detail** section. ## Tools Used Manual review ## Recommended Mitigation Steps Modify function LBRouter._swapSupportingFeeOnTransferTokens as follow if (_binStep == 0) { (uint256 _reserve0, uint256 _reserve1, ) = IJoePair(_pair).getReserves(); if (_token < _tokenNext) { uint256 _balance = _token.balanceOf(_pair); // fix here uint256 _amountOut = (_reserve1 * (_balance - _reserve0) * 997) / (_balance * 997 + 3 * _reserve0); IJoePair(_pair).swap(0, _amountOut, _recipient, ""); } else { uint256 _balance = _token.balanceOf(_pair); // fix here uint256 _amountOut = (_reserve0 * (_balance - _reserve1) * 997) / (_balance * 997 + 3 * _reserve1); IJoePair(_pair).swap(_amountOut, 0, _recipient, ""); } } else { ILBPair(_pair).swap(_tokenNext == ILBPair(_pair).tokenY(), _recipient); } --- The text was updated successfully, but these errors were encountered: All reactions