zzykxx - Use of `transferFrom()` instead of `safeTransferFrom()`

[sherlock-admin]

zzykxx

medium

Use of transferFrom() instead of safeTransferFrom()

Summary

transferFrom() is used without checking the return value.

Vulnerability Detail

The topUp() function transfers the rewardToken from the source to the staking reward contracts, to do so it uses the transferFrom() function. Tokens might fail to be transferred (ex. not enough are approved) and return false, but this is never checked in the code.

Impact

If the topUp() function might not revert on a token transfer fail and will continue execution calling notifyRewardAmount() on the staking contracts.

Code Snippet

• transferFrom()

  Tool used

Manual Review

Recommendation

Use openzeppelin safeTransferFrom() instead of transferFrom(), if a transfer fail the function will always revert.

Duplicate of #8

[sherlock-admin2]

1 comment(s) were left on this issue during the judging contest.

takarez commented:

valid because { This is valid and a dupp of 008}