J4de - Malicious `_swapAddress` could steal all user funds

[sherlock-admin]

J4de

high

Malicious _swapAddress could steal all user funds

Summary

Malicious _swapAddress could steal all user funds

Vulnerability Detail

Take _opentrade function as an example.

File: MarginTrading.sol
264         if (_swapParams.length > 0) {
265             // approve to swap route
266             for (uint256 i = 0; i < _swapApproveToken.length; i++) {
267                 IERC20(_swapApproveToken[i]).approve(_swapApproveTarget, type(uint256).max);
268             }
269
270  >>         (bool success,) = _swapAddress.call(_swapParams);
271             require(success, "dodoswap fail");
272         }
273         uint256[] memory _tradeAmounts = new uint256[](_tradeAssets.length);
274         for (uint256 i = 0; i < _tradeAssets.length; i++) {
275  >>         _tradeAmounts[i] = IERC20(_tradeAssets[i]).balanceOf(address(this));
276             _lendingPoolDeposit(_tradeAssets[i], _tradeAmounts[i], 1);
277         }

Line 270 will do token swap. After the swap, it call the _lendingPoolDeposit function to deposit the exchanged token into the pool. Line 275 uses balanceOf(address(this) as the deposit amount, which makes any deposit amount legal. In other words, even if _swapAddress does not return any token, MarginTrading.sol contract can still continue to execute smoothly.

Impact

Malicious _swapAddress can steal all user funds

Code Snippet

https://github.com/sherlock-audit/2023-05-dodo/blob/main/dodo-margin-trading-contracts/contracts/marginTrading/MarginTrading.sol#L275

https://github.com/sherlock-audit/2023-05-dodo/blob/main/dodo-margin-trading-contracts/contracts/marginTrading/MarginTrading.sol#L318

https://github.com/sherlock-audit/2023-05-dodo/blob/main/dodo-margin-trading-contracts/contracts/marginTrading/MarginTrading.sol#L322

Tool used

Manual Review

Recommendation

Use user-specified deposit amount instead of balanceOf(address(this)

Duplicate of #104