It is good to add a require() statement that checks the return value of token transfers or to use something like OpenZeppelin’s safeTransfer/safeTransferFrom unless one is sure the given token reverts in case of a failure. Failure to do so will cause silent failures of transfers and affect token accounting in contract.
Vulnerability Detail
Some tokens (like USDT) don't correctly implement the EIP20 standard and their transfer/transferFrom function return void instead of a successful boolean. Calling these functions with the correct EIP20 function signatures will always revert.
Impact
Tokens that don't correctly implement the latest EIP20 spec, like USDT, will be unusable in the protocol as they revert the transaction because of the missing return value. As there is a xToken with USDT as the underlying issue directly applies to the protocol.
SaharDevep
medium
Unchecked return value of transfer()
sahardevep
medium
Unchecked return value of transfer()
Summary
It is good to add a require() statement that checks the return value of token transfers or to use something like OpenZeppelin’s safeTransfer/safeTransferFrom unless one is sure the given token reverts in case of a failure. Failure to do so will cause silent failures of transfers and affect token accounting in contract.
Vulnerability Detail
Some tokens (like USDT) don't correctly implement the EIP20 standard and their transfer/transferFrom function return void instead of a successful boolean. Calling these functions with the correct EIP20 function signatures will always revert.
Impact
Tokens that don't correctly implement the latest EIP20 spec, like USDT, will be unusable in the protocol as they revert the transaction because of the missing return value. As there is a xToken with USDT as the underlying issue directly applies to the protocol.
Code Snippet
https://github.com/sherlock-audit/2023-05-dodo/blob/8e6dceb9f3f5cb42fe591d3ef25b002d9916ac71/dodo-margin-trading-contracts/contracts/marginTrading/MarginTrading.sol#L212
https://github.com/sherlock-audit/2023-05-dodo/blob/8e6dceb9f3f5cb42fe591d3ef25b002d9916ac71/dodo-margin-trading-contracts/contracts/marginTrading/MarginTrading.sol#L341
https://github.com/sherlock-audit/2023-05-dodo/blob/8e6dceb9f3f5cb42fe591d3ef25b002d9916ac71/dodo-margin-trading-contracts/contracts/marginTrading/MarginTradingFactory.sol#L218
Tool used
Manual Review
Recommendation
Consider using safeTransfer/safeTransferFrom or require() consistently.