Users lose the receiving of remaining tokens

Summary

The return amount should be sent back to msg.sender instead of the recipient.

Vulnerability Detail

The to input argument in the wrapTokensAndaddLiquidity function in the JalaMasterRouter contract represents the recipient address where msg.sender wants the liquidity tokens to be minted.

At the beginning of the function, msg.sender transfers tokens to JalaMasterRouter before calling the router to add liquidity.

TransferHelper.safeTransferFrom(tokenA, msg.sender, address(this), amountADesired);
TransferHelper.safeTransferFrom(tokenB, msg.sender, address(this), amountBDesired);

At the end of the function, the remaining tokens, after providing liquidity, are unwrapped and transferred back to the recipient.

  _unwrapAndTransfer(wrappedTokenA, to);
  _unwrapAndTransfer(wrappedTokenB, to);

In the scenario where to = msg.sender, the remaining tokens are returned to msg.sender (their original source). However, in the case where to != msg.sender, the tokens are transferred to another address. This address may be one where msg.sender does not want to go or the toaddress is an EOA/smart wallet that can not send tokens back to msg.sender. In the second case, msg.sender will lose the remaining tokens.

The to address should only receive LP tokens and the remaining tokens should be returned to msg.sender.

Impact

Users will lose the receiving of remaining tokens.

Code Snippet

https://github.com/sherlock-audit/2024-02-jala-swap/blob/main/jalaswap-dex-contract/contracts/JalaMasterRouter.sol#L67-L68

https://github.com/sherlock-audit/2024-02-jala-swap/blob/main/jalaswap-dex-contract/contracts/JalaMasterRouter.sol#L98

Tool used

Manual Review

Recommendation

Send remaining tokens back to the msg.sender.

sherlock-audit / 2024-02-jala-swap-judging

gkrastenovaudit - Users lose the receiving of remaining tokens #249