Omniscia SaucerSwap Labs Audit

UniswapV2Router02 Manual Review Findings

UniswapV2Router02 Manual Review Findings

UVR-01M: Potential Arithmetic Underflow

TypeSeverityLocation
Mathematical OperationsUniswapV2Router02.sol:L96

Description:

The msg.value of the UniswapV2Router02::addLiquidityETHNewPool is not guaranteed to be less than feeInTinybars, potentially leading to an underflow in the UniswapV2Router02::_addLiquidity function argument calculation.

Impact:

While an underflow can occur, it would result in the code not executing as the amountToken and amountETH the function would calculate would be exorbitant and unfulfillable by the msg.sender.

Example:

contracts/UniswapV2Router02.sol
83function addLiquidityETHNewPool(
84    address token,
85    uint amountTokenDesired,
86    uint amountTokenMin,
87    uint amountETHMin,
88    address to,
89    uint deadline
90) external virtual payable override ensure(deadline) returns (uint amountToken, uint amountETH, uint liquidity) {
91    require (IUniswapV2Factory(factory).getPair(token, whbar) == address(0), "UniswapV2Router: POOL ALREADY EXISTS");
92    uint256 feeInTinybars = tinycentsToTinybars(IUniswapV2Factory(factory).pairCreateFee());
93

94    IUniswapV2Factory(factory).createPair{value: feeInTinybars}(token, whbar);
95

96    (amountToken, amountETH) = _addLiquidity(token, whbar, amountTokenDesired, msg.value - feeInTinybars, amountTokenMin, amountETHMin);
97    address pair = UniswapV2Library.pairFor(factory, token, whbar);
98    
99    safeTransferToken(
100        token, msg.sender, pair, amountToken
101    );
102    IWHBAR(WHBAR).deposit{value: amountETH}(msg.sender, pair);
103    liquidity = IUniswapV2Pair(pair).mint(to);
104    if (msg.value - feeInTinybars > amountETH) TransferHelper.safeTransferETH(msg.sender, msg.value - feeInTinybars - amountETH);
105}

Recommendation:

We advise the code to require that the msg.value is at least greater-than the feeInTinybars, preventing this underflow from occurring and rendering the operations in UniswapV2Router02::addLiquidityETHNewPool safe.

Alleviation (a2c5a0b913):

The SaucerSwap team stated that they believe this to be incorrect. We would like to note that the UniswapV2Router02::addLiquidityETHNewPool function can invoke UniswapV2Factory::createPair with a value of feeInTinybars even if msg.value is less than that. This is possible because such native calls can tap into the native balance of the contract, not only the msg.value.

As an example, a user can send the feeInTinybars value directly to the UniswapV2Router02 contract and then invoke UniswapV2Router02::addLiquidityETHNewPool with a msg.value of 0. This would cause L94 (L95 in the latest commit hash) to successfully execute and thus permit the underflow.

As such, we still advise the require check to be introduced.

Alleviation (fd26dbdf6d):

The SaucerSwap team has introduced the recommended require check in the UniswapV2Router02::addLiquidityETHNewPool function as advised, rendering this exhibit properly alleviated.

UVR-02M: Potentially Discrepant Exchange Flow

TypeSeverityLocation
Logical FaultUniswapV2Router02.sol:L414, L418

Description:

The UniswapV2Router02::swapExactTokensForETHSupportingFeeOnTransferTokens function will perform the sequences of swaps defined in the path argument with the final recipient being the msg.sender. Afterwards, it will perform a withdrawal operation from the msg.sender to the to address, in turn requiring an approval from the msg.sender to the WHBAR contract (Operator) of the whbar asset (wHBAR token).

Impact:

The current swapping methodology will require an approval of the msg.sender to the WHBAR contract in addition to the approval required on the UniswapV2Router02 which is discrepant with its normal operation.

Example:

contracts/UniswapV2Router02.sol
397function swapExactTokensForETHSupportingFeeOnTransferTokens(
398    uint amountIn,
399    uint amountOutMin,
400    address[] calldata path,
401    address to,
402    uint deadline
403)
404    external
405    virtual
406    override
407    ensure(deadline)
408{
409    require(path[path.length - 1] == whbar, 'UniswapV2Router: INVALID_PATH');
410    uint startAmount = IERC20(whbar).balanceOf(msg.sender);
411    safeTransferToken(
412        path[0], msg.sender, UniswapV2Library.pairFor(factory, path[0], path[1]), amountIn
413    );
414    _swapSupportingFeeOnTransferTokens(path, msg.sender);
415    uint endAmount = IERC20(whbar).balanceOf(msg.sender);
416    uint amountOut = endAmount.sub(startAmount);
417    require(amountOut >= amountOutMin, 'UniswapV2Router: INSUFFICIENT_OUTPUT_AMOUNT');
418    IWHBAR(WHBAR).withdraw(msg.sender, to, amountOut);
419}

Recommendation:

We advise the code to revise its methodology and to instead set itself as the output of the swaps, unwrapping the whbar token via the WHBAR contract on itself similarly to UniswapV2Router02::removeLiquidityETH.

Alleviation (a2c5a0b913a7ddc21ff96f97fa51f2820a5da7ec):

The code has been revised as advised, setting the router itself as the recipient of the swap chain and unwrapping the asset to the msg.sender thus ensuring that no superfluous approvals are needed.

UVR-03M: Incorrect Recipient of ETH Outputs

TypeSeverityLocation
Logical FaultUniswapV2Router02.sol:L302, L319

Description:

The UniswapV2Router02::swapExactTokensForETH and UniswapV2Router02::swapTokensForExactETH functions presently misbehave as they set the output of the UniswapV2Router02::_swap to the to address, performing a withdrawal of the whbar asset on the msg.sender incorrectly.

Impact:

The code presently misbehaves by performing a swap of the amount directly to the to target and providing them with double the output amount in the form of the whbar asset unwrapped via the WHBAR contract.

Example:

contracts/UniswapV2Router02.sol
288function swapTokensForExactETH(uint amountOut, uint amountInMax, address[] calldata path, address to, uint deadline)
289    external
290    virtual
291    override
292    ensure(deadline)
293    returns (uint[] memory amounts)
294{
295    require(path[path.length - 1] == whbar, 'UniswapV2Router: INVALID_PATH');
296    amounts = UniswapV2Library.getAmountsIn(factory, amountOut, path);
297    require(amounts[0] <= amountInMax, 'UniswapV2Router: EXCESSIVE_INPUT_AMOUNT');
298    safeTransferToken(
299        path[0], msg.sender, UniswapV2Library.pairFor(factory, path[0], path[1]), amounts[0]
300    );
301    
302    _swap(amounts, path, to);       
303    IWHBAR(WHBAR).withdraw(msg.sender, to, amounts[amounts.length - 1]);
304}
305function swapExactTokensForETH(uint amountIn, uint amountOutMin, address[] calldata path, address to, uint deadline)
306    external
307    virtual
308    override
309    ensure(deadline)
310    returns (uint[] memory amounts)
311{
312    require(path[path.length - 1] == whbar, 'UniswapV2Router: INVALID_PATH');
313    amounts = UniswapV2Library.getAmountsOut(factory, amountIn, path);
314    require(amounts[amounts.length - 1] >= amountOutMin, 'UniswapV2Router: INSUFFICIENT_OUTPUT_AMOUNT');
315

316    safeTransferToken(
317        path[0], msg.sender, UniswapV2Library.pairFor(factory, path[0], path[1]), amounts[0]
318    );
319     _swap(amounts, path, to);       
320    IWHBAR(WHBAR).withdraw(msg.sender, to, amounts[amounts.length - 1]);
321}

Recommendation:

We advise them to perform the swap with an output to the contract itself, unwrapping the whbar asset via the WHBAR operator locally similarly to UniswapV2Router02::removeLiquidityETH.

Alleviation (a2c5a0b913a7ddc21ff96f97fa51f2820a5da7ec):

The recipient of the swaps has been corrected to the contract itself, permitting it to unwrap the whbar asset correctly.

Prev

HederaTokenService.sol (HTS-M)

Next

Bits.sol (BST-C)

On this page