code-423n4 / 2023-01-ondo-findings

0 stars 0 forks source link

Use safeTransefer and safeTransferfrom #361

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-01-ondo/blob/f3426e5b6b4561e09460b2e6471eb694efdd6c70/contracts/lending/tokens/cToken/CErc20.sol#L150 https://github.com/code-423n4/2023-01-ondo/blob/f3426e5b6b4561e09460b2e6471eb694efdd6c70/contracts/lending/tokens/cToken/CErc20.sol#L160 https://github.com/code-423n4/2023-01-ondo/blob/f3426e5b6b4561e09460b2e6471eb694efdd6c70/contracts/lending/tokens/cToken/CErc20.sol#L193 https://github.com/code-423n4/2023-01-ondo/blob/f3426e5b6b4561e09460b2e6471eb694efdd6c70/contracts/lending/tokens/cToken/CErc20.sol#L201 https://github.com/code-423n4/2023-01-ondo/blob/f3426e5b6b4561e09460b2e6471eb694efdd6c70/contracts/lending/tokens/cToken/CErc20.sol#L236 https://github.com/code-423n4/2023-01-ondo/blob/f3426e5b6b4561e09460b2e6471eb694efdd6c70/contracts/lending/tokens/cToken/CErc20.sol#L241

Vulnerability details

Impact and Recommended Mitigation Steps

CErc20.sol

Transfers are not safe because there is no checking whether the return value is true/false without use of Openzeppelins safeTransfer/safeTransferFrom. Should use the @openzeppelin-contracts-upgradeable/contracts/token/ERC20/utils/SafeERC20Upgradeable.sol and Ownable.sol Because it includes safeTransefer and safeTransferfrom Function sweepToken, line 150 should be Ownable Line 160 is token.transfer(admin, balance); Should be token.safeTransfer(admin, balance);

Function function doTransferIn should be is Ownable Furthermore line 193 is: token.transferFrom(from, address(this), amount); Should be token.safetransferFrom(from, address(this), amount);

Same changes should be done to function function doTransferOut

Tools Used

none

c4-judge commented 1 year ago

trust1995 marked the issue as unsatisfactory: Out of scope