Using "transfer/transferFrom" instead of "safeTransfer/safeTransferFrom"

[code423n4]

Lines of code

https://github.com/code-423n4/2022-07-fractional/blob/e2c5a962a94106f9495eb96769d7f60f7d5b14c9/src/modules/protoforms/BaseVault.sol#L65

Vulnerability details

Impact

Using transfer and transferFrom instead of their safe alternatives may result in transactions fail silently.

Proof of Concept

Using token transferFrom functions instead of safeTransferFrom (BaseVault.sol#L65) which is discouraged and can cause tokens to be stuck in the case of the transaction not reverting on failed transfers. There’s also precedents of this vulnerability as seen here
code-423n4/2022-01-trader-joe-findings#12

Tools Used

Manual code review

Recommended Mitigation Steps

We suggest you to check all of your contracts and fix this issue by implementing safeTransfer and safeTransferFrom instead of transfer and transferFrom where applicable.

[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 #450