Unchecked transfer

[code423n4]

Lines of code

https://github.com/code-423n4/2022-07-fractional/blob/main/src/modules/protoforms/BaseVault.sol#L65 https://github.com/code-423n4/2022-07-fractional/blob/main/src/references/TransferReference.sol#L22 https://github.com/code-423n4/2022-07-fractional/blob/main/src/utils/SafeSend.sol#L33

Vulnerability details

Description

The return value of an external transfer/transferFrom call is not checked

Impact

There are some tokens that do not revert on failure but return false instead, if such token is used, the return value won't be checked and the function won't revert even if the transfer fails. The remaining code will still be executed and expect that the transaction was successful.

Therefore it is recommended to ensure the return value of transfer is checked.

Link to code

https://github.com/code-423n4/2022-07-fractional/blob/main/src/modules/protoforms/BaseVault.sol#L65 https://github.com/code-423n4/2022-07-fractional/blob/main/src/references/TransferReference.sol#L22 https://github.com/code-423n4/2022-07-fractional/blob/main/src/utils/SafeSend.sol#L33

Tools Used

Slither

Recommended Mitigation Steps

Use SafeERC20, or ensure that the transfer/transferFrom return value is checked.

[stevennevins]

Duplicate of #312

[HardlyDifficult]

Agree with the sponsor that this is a non-critical best practice. Transfers may fail but no events are emitted, balance doesn't change, and no other negative consequences were identified.

Merging with the warden's QA report #631