DeFiFoundry
20,000 USDC
View results
Submission Details
Severity: medium
Invalid

Inconsistent Use of `safeTransfer` and `transfer` and `safeTransferFrom` and `transferFrom`

Summary

The FjordStaking contract inconsistently uses safeTransfer and transfer and safeTransferFrom and transferFrom functions for ERC20 token interactions. This inconsistency can lead to silent failures in token transfers, potentially resulting in loss of funds or incorrect contract state.

Vulnerability Details

The contract uses both SafeERC20 functions (safeTransfer, safeTransferFrom) and regular ERC20 functions (transfer, transferFrom)

  1. In the _unstakeVested function:

  2. In the _unstakeVested function:

  3. In the unstakeAll function:

fjordToken.transfer(msg.sender, totalStakedAmount);
sablier.transferFrom({ from: msg.sender, to: address(this), tokenId: _streamID });
sablier.transferFrom({ from: address(this), to: streamOwner, tokenId: _streamID });

The regular transfer and transferFrom function doesn't revert on failure for all ERC20 implementations. Some tokens return a boolean to indicate success or failure. If the contract doesn't check this return value, it might not detect failed transfers.

Impact

Failed transfers might go unnoticed, leading to users not receiving their tokens, and the contract might update its state assuming a successful transfer, even if the transfer has failed.

Tools Used

Manual review

Recommendations

Replace all the transferand transferFromfunction with SafeERC20 function like other functions in the contract to keep consistency across the contract and the protocol in general, and protect against silent failures

Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Known issue

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.