31Third security record

The project uses Solidity version `0.8.16`, which is not recommended for deployment.

Some tokens can implement hooks or fee systems that get executed when transferred. That means that the amount pulled by the smart contract (calling `transferFrom()`) and the actual amount increased to the balance are not the same. Handling the accounting of these tokens can be complex and can lead to unexpected behavior. <br><br> 31Third is not compatible with these tokens. This is a usual approach in many major DeFi protocols by design and not a security issue. <br><br> In `_claimAndApproveFromToken()` the following condition is checked following the transfer of funds from the caller: ``` if (fromBalanceAfter < fromBalanceBefore + _trade.fromAmount) { revert NotEnoughClaimed(_trade, _trade.fromAmount, fromBalanceAfter - fromBalanceBefore); } ``` If a token was being used that takes a fee on transfer, the trade would always revert.

The `reduceMaxBasisPoints()` allows the maximum fee to be set to a lower value. It is important to note that once the maximum fee limit is reduced, it cannot be raised again. If for some reason, the maximum fee was set to zero, it would be impossible to charge fees for trades.

Smart contracts will often have `owner` variables to designate the person with special privileges to make modifications to the smart contract. <br><br> In the case where a malicious actor obtains control of the owner's wallet, privileged functionality can be exploited in a way that harms future users. Specifically, a malicious actor could call `ExchangeAdapterRegistry.editAdapter()` to change the adapter address to a malicious contract for a previously established exchange. After the exchange adapter is updated, users that call `BatchTrade.batchTrade()` using the now compromised exchange adapter are subject to having their funds stolen by the malicious adapter contract. The following are all other cases of owner privileges: - `BatchTrade.activate()` and `BatchTrade.deactivate()` allows the owner to start/stop trading. - `BatchTrade.addFeelessWallet()`, `BatchTrade.addFeelessWallets()`, `BatchTrade.removeFeelessWallet()`, and `BatchTrade.removeFeelessWallets()` allows the owner to add or remove wallets that can trade without paying fees. - `BatchTrade.updateFeeRecipient()`, `BatchTrade.updateBasisPoints()`, and `BatchTrade.reduceMaxBasisPoints()` allows the owner to update the trade fee and the fee recipient.

The `BatchTrade.batchTrade()` function allows a user to pass in arbitrary call data to be executed on an exchange contract. The function invoked on the exchange contract may return data which is collected as shown below: ``` (bool targetSuccess, bytes memory result) = _target.call{value: _value}( _data ); ``` However, no validation is done on the `result` data obtained. The only action taken is to log the data in an event upon trade failure: `emit TradeFailedReason(**msg.sender**, _trade.from, _trade.to, result);`. Depending on the data returned from the exchange, it may be worth performing validation checks upon the return data. This validation would be specific to the exchange, and so would likely belong on the appropriate adapter contract.