[NAZ-M3] Use `safeTransfer()/safeTransferFrom()` instead of `transfer()/transferFrom()`

[code423n4]

Lines of code

https://github.com/code-423n4/2022-09-y2k-finance/blob/main/src/Vault.sol#L228 https://github.com/code-423n4/2022-09-y2k-finance/blob/main/src/Vault.sol#L231 https://github.com/code-423n4/2022-09-y2k-finance/blob/main/src/Vault.sol#L365

Vulnerability details

Impact

It is a good idea to add a require() statement that checks the return value of ERC20 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.

However, using require() to check transfer return values could lead to issues with non-compliant ERC20 tokens which do not return a boolean value. Therefore, it's highly advised to use OpenZeppelin’s safeTransfer()/safeTransferFrom().

Tools Used

Manual Review

Recommended Mitigation Steps

Consider using safeTransfer()/safeTransferFrom() instead of transfer()/transferFrom().

[MiguelBits]

not implementing this as we only use WETH which does not need safetransfers

[HickupHH3]

dup #499