M01 TOKEN TRANSFERS DO NOT VERIFY THAT THE TOKENS WERE SUCCESSFULLY TRANSFERRED

[code423n4]

Lines of code

https://github.com/code-423n4/2022-09-y2k-finance/tree/main/src/SemiFungibleVault.sol#L99
https://github.com/code-423n4/2022-09-y2k-finance/tree/main/src/SemiFungibleVault.sol#L127
https://github.com/code-423n4/2022-09-y2k-finance/tree/main/src//rewards/StakingRewards.sol#L99
https://github.com/code-423n4/2022-09-y2k-finance/tree/main/src//rewards/StakingRewards.sol#L122
https://github.com/code-423n4/2022-09-y2k-finance/tree/main/src//rewards/StakingRewards.sol#L136
https://github.com/code-423n4/2022-09-y2k-finance/tree/main/src//rewards/StakingRewards.sol#L221
https://github.com/code-423n4/2022-09-y2k-finance/tree/main/src/SemiFungibleVault.sol#L94
https://github.com/code-423n4/2022-09-y2k-finance/tree/main/src/SemiFungibleVault.sol#L127

Vulnerability details

problem

Some tokens (like zrx) do not revert the transaction when the transfer/transferfrom fails and return false, which requires us to check the return value after calling the transfer/transferfrom function.

In the purchase functions of MiniSales and TokenShop contracts, if _paymentToken is such a token, the user can purchase _saleToken and NFT without spending any tokens.

In the claim function of the Vesting contract, if _token is such a token, the user may lose his vested tokens.

prof

src/SemiFungibleVault.sol, 94, b' asset.safeTransferFrom(msg.sender, address(this), assets);'
src/SemiFungibleVault.sol, 127, b' asset.safeTransfer(receiver, assets);'
src/rewards/StakingRewards.sol, 99, b' stakingToken.safeTransferFrom('
src/rewards/StakingRewards.sol, 122, b' stakingToken.safeTransferFrom('
src/rewards/StakingRewards.sol, 136, b' rewardsToken.safeTransfer(msg.sender, reward);'
src/rewards/StakingRewards.sol, 221, b' ERC20(tokenAddress).safeTransfer(owner, tokenAmount);'
src/SemiFungibleVault.sol, 94, b' asset.safeTransferFrom(msg.sender, address(this), assets);'
src/SemiFungibleVault.sol, 127, b' asset.safeTransfer(receiver, assets);'

mitigation

We should check the return value of the function.

[HickupHH3]

dup #499