Fjord Token Staking

Impact

ERC20 implementations are not always consistent. Some implementations of transfer and transferFrom could return ‘false’ on failure instead of reverting. It is safer to wrap such calls into require() statements or use safe wrapper functions implementing return value/data checks to handle these failures. For reference, see similar Medium-severity finding from Consensys Diligence Audit of Aave Protocol V2: https://consensys.net/diligence/audits/2020/09/aave-protocol-v2/#unhandled-return-values-of-transfer-and-transferfrom

Proof of Concept

One scenario

Alice has staked tokens across multiple epochs in the contract. FjordStaking is the contract that handles staking and rewards. Alice has staked tokens in several epochs. The contract is designed to handle unstaking and reward claims.

Step 1: Alice calls the unstakeAll() function to unstake from all epochs and claim all pending rewards.

Step 2: Suppose the ERC20 token fjordToken has an issue where its transfer() function does not revert on failure but returns false. If the contract does not have sufficient balance or other conditions fail, fjordToken.transfer() will fail silently.

Step 3: Alice assumes that the tokens have been successfully transferred based on the event emitted. However, if the transfer() function fails silently, Alice does not receive any tokens, and the contract state remains inconsistent.

Step 4: Alice is unable to recover her unstaked amount because the transfer() call did not succeed, and the function did not handle the failure properly. This can lead to unexpected behavior and fund loss for the user.

Step 5: When Alice calls unstakeAll(), she expects to receive the total unstaked amount in tokens. If the transfer() fails and does not revert, Alice does not receive any tokens, leading to potential loss of her funds or unexpected results.

• https://github.com/Cyfrin/2024-08-fjord/blob/0312fa9dca29fa7ed9fc432fdcd05545b736575d/src/FjordStaking.sol#L600

• https://github.com/Cyfrin/2024-08-fjord/blob/0312fa9dca29fa7ed9fc432fdcd05545b736575d/src/FjordAuction.sol#L151C9-L151C69

• https://github.com/Cyfrin/2024-08-fjord/blob/0312fa9dca29fa7ed9fc432fdcd05545b736575d/src/FjordAuction.sol#L174

• https://github.com/Cyfrin/2024-08-fjord/blob/0312fa9dca29fa7ed9fc432fdcd05545b736575d/src/FjordAuction.sol#L193C13-L193C55

• https://github.com/Cyfrin/2024-08-fjord/blob/0312fa9dca29fa7ed9fc432fdcd05545b736575d/src/FjordAuction.sol#L220

Tools Used

Manual Analysis

Recommendation

Check the return value and revert on 0/false or use safeERC20 library.