code-423n4 / 2022-04-dualityfocus-findings

1 stars 0 forks source link

Use safeTransfer/safeTransferFrom consistently instead of transfer/transferFrom #2

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-04-dualityfocus/blob/main/contracts/compound_rari_fork/CErc20.sol#L141-L145

Vulnerability details

Impact

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.

Proof of Concept

https://github.com/code-423n4/2022-04-dualityfocus/blob/main/contracts/compound_rari_fork/CErc20.sol#L141-L145 https://github.com/code-423n4/2022-04-dualityfocus/blob/main/contracts/vault_and_oracles/FlashLoan.sol#L57

Tools Used

None

Recommended Mitigation Steps

Consider using safeTransfer/safeTransferFrom or require() consistently.

0xdramaone commented 2 years ago

Duplicate of #40

JeeberC4 commented 2 years ago

Adding to warden's QA Report #22