code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2022-12-Stealth-Project/blob/main/maverick-v1/contracts/models/Pool.sol#L302 https://github.com/code-423n4/2022-12-Stealth-Project/blob/main/maverick-v1/contracts/interfaces/ISwapCallback.sol#L4-L6 https://github.com/code-423n4/2022-12-Stealth-Project/blob/main/router-v1/contracts/Router.sol#L88-L119

Vulnerability details

Impact

When a trader calls swap() in Pool.sol, an external call ISwapCallback(msg.sender).swapCallback(amountIn, amountOut, data) is invoked making sure the callback is going to send the amountIn of tokenA or tokenB to the pool. While it is only required that the trader has transferred at least the amountToPay (amountIn), he could end up transferring more than is required which is acceptably fine. However, under this situation, this overpaying for the swap will not be reimbursed to the trader. Neither will the trader receive additional output token corresponding to this surplus of tokens sent in.

Proof of Concept

File: Pool.sol#L302

        ISwapCallback(msg.sender).swapCallback(amountIn, amountOut, data);

As can be seen from the code block above, msg.sender is Pool.sol that will be recipient in pay() internally called by swapCallback() in Router.sol:

File: Router.sol

88:    function pay(IERC20 token, address payer, address recipient, uint256 value) internal {

114:                pay(tokenOut, data.payer, msg.sender, amountToPay);

117:            pay(tokenIn, data.payer, msg.sender, amountToPay);

Additionally, it is noted that the function logic of Pool.sol does not implement reimbursement of the extra tokens sent in by traders.

Tools Used

Manual inspection

Recommended Mitigation Steps

Considering refunding the trader with the additional tokens sent in or having the following code refactored as follows:

File: Pool.sol#L303

-        require(previousBalance + amountIn <= (tokenAIn ? _tokenABalance() : _tokenBBalance()), "S");
+        require(previousBalance + amountIn == (tokenAIn ? _tokenABalance() : _tokenBBalance()), "S");