Anyone can sell other users' tokens as `fromToken`, and get the `toToken`'s themselves due to `decodeData.payer` is never checked.
Anyone can sell other users' tokens as `fromToken`, and get the toToken's themselves due to `decodeData.payer` is never checked.\\nLet's examine the token-selling process and the transaction flow.\\nThe user will initiate the transaction with the `sellTokens()` method in the `D3Proxy.sol` contract, and provide multiple inputs like `pool`, `fromToken`, `toToken`, `fromAmount`, `data` etc.\\n```\\n// File: D3Proxy.sol\\n    function sellTokens(\\n        address pool,\\n        address to,\\n        address fromToken,\\n        address toToken,\\n        uint256 fromAmount,\\n        uint256 minReceiveAmount,\\n        bytes calldata data,\\n        uint256 deadLine\\n    ) public payable judgeExpired(deadLine) returns (uint256 receiveToAmount) {\\n        if (fromToken == _ETH_ADDRESS_) {\\n            require(msg.value == fromAmount, "D3PROXY_VALUE_INVALID");\\n            receiveToAmount = ID3MM(pool).sellToken(to, _WETH_, toToken, fromAmount, minReceiveAmount, data);\\n        } else if (toToken == _ETH_ADDRESS_) {\\n            receiveToAmount =\\n                ID3MM(pool).sellToken(address(this), fromToken, _WETH_, fromAmount, minReceiveAmount, data);\\n            _withdrawWETH(to, receiveToAmount);\\n            // multicall withdraw weth to user\\n        } else {\\n            receiveToAmount = ID3MM(pool).sellToken(to, fromToken, toToken, fromAmount, minReceiveAmount, data);\\n        }\\n    }\\n```\\n\\nAfter some checks, this method in the `D3Proxy.sol` will make a call to the `sellToken()` function in the pool contract (inherits D3Trading.sol). After this call, things that will happen in the pool contract are:\\nTransferring the toToken's to the "to" address (with _transferOut)\\nMaking a callback to `D3Proxy` contract to deposit fromToken's to the pool. (with IDODOSwapCallback(msg.sender).d3MMSwapCallBack)\\nChecking the pool balance and making sure that the fromToken's are actually deposited to the pool. (with this line: IERC20(fromToken).balanceOf(address(this)) - state.balances[fromToken] >= fromAmount)\\n```\\n// File: D3Trading.sol\\n// Method: sellToken()\\n108.-->  _transferOut(to, toToken, receiveToAmount);\\n109.\\n110.     // external call & swap callback\\n111.-->  IDODOSwapCallback(msg.sender).d3MMSwapCallBack(fromToken, fromAmount, data);\\n112.     // transfer mtFee to maintainer\\n113.     _transferOut(state._MAINTAINER_, toToken, mtFee);\\n114.\\n115.     require(\\n116.-->      IERC20(fromToken).balanceOf(address(this)) - state.balances[fromToken] >= fromAmount,\\n117.         Errors.FROMAMOUNT_NOT_ENOUGH\\n118.     );\\n```\\n\\nThe source of the vulnerability is the `d3MMSwapCallBack()` function in the `D3Proxy`. It is called by the pool contract with the `fromToken`, `fromAmount` and `data` inputs to make a `fromToken` deposit to the pool.\\n```\\n//File: D3Proxy.sol \\n    /// @notice This callback is used to deposit token into D3MM\\n    /// @param token The address of token\\n    /// @param value The amount of token need to deposit to D3MM\\n    /// @param _data Any data to be passed through to the callback\\n    function d3MMSwapCallBack(address token, uint256 value, bytes calldata _data) external override {\\n        require(ID3Vault(_D3_VAULT_).allPoolAddrMap(msg.sender), "D3PROXY_CALLBACK_INVALID");\\n        SwapCallbackData memory decodeData;\\n        decodeData = abi.decode(_data, (SwapCallbackData));\\n-->     _deposit(decodeData.payer, msg.sender, token, value);\\n    }\\n```\\n\\nAn attacker can create a `SwapCallbackData` struct with any regular user's address, encode it and pass it through the `sellTokens()` function, and get the toToken's.\\nYou can say that `_deposit()` will need the payer's approval but the attackers will know that too. A regular user might have already approved the pool & proxy for the max amount. Attackers can easily check any token's allowances and exploit already approved tokens. Or they can simply watch the mempool and front-run any normal seller right after they approve but before they call the `sellTokens()`.
I would recommend to check if the `decodeData.payer == msg.sender` in the beginning of the `sellTokens()` function in `D3Proxy` contract. Because msg.sender will be the pool's address if you want to check it in the `d3MMSwapCallBack()` function, and this check will not be valid to see if the payer is actually the seller.\\nAnother option might be creating a local variable called "seller" and saving the msg.sender value when they first started the transaction. After that make `decodeData.payer == seller` check in the `d3MMSwapCallBack()`.
An attacker can sell any user's tokens and steal their funds.
```\\n// File: D3Proxy.sol\\n    function sellTokens(\\n        address pool,\\n        address to,\\n        address fromToken,\\n        address toToken,\\n        uint256 fromAmount,\\n        uint256 minReceiveAmount,\\n        bytes calldata data,\\n        uint256 deadLine\\n    ) public payable judgeExpired(deadLine) returns (uint256 receiveToAmount) {\\n        if (fromToken == _ETH_ADDRESS_) {\\n            require(msg.value == fromAmount, "D3PROXY_VALUE_INVALID");\\n            receiveToAmount = ID3MM(pool).sellToken(to, _WETH_, toToken, fromAmount, minReceiveAmount, data);\\n        } else if (toToken == _ETH_ADDRESS_) {\\n            receiveToAmount =\\n                ID3MM(pool).sellToken(address(this), fromToken, _WETH_, fromAmount, minReceiveAmount, data);\\n            _withdrawWETH(to, receiveToAmount);\\n            // multicall withdraw weth to user\\n        } else {\\n            receiveToAmount = ID3MM(pool).sellToken(to, fromToken, toToken, fromAmount, minReceiveAmount, data);\\n        }\\n    }\\n```\\n