Liquid Staking

Stakelink
DeFiHardhatOracle
50,000 USDC
View results
Submission Details
Severity: medium
Invalid

contracts/core/rewardsPools/RewardsPoolWSD.sol

The provided Solidity code implements a rewards pool contract for a wrapped liquid staking token. While the code is generally structured well, there are several potential vulnerabilities and considerations you should be aware of:

1. Reentrancy Attacks

Although the use of SafeERC20 provides some safety during token transfers, the _withdraw function can be susceptible to reentrancy attacks due to the external call to wsdToken.unwrap(toWithdraw) and the transfer to _account. To mitigate this:

  • Use the Checks-Effects-Interactions pattern. Update state variables (like userRewards[_account] and totalRewards) before making any external calls.

2. Incorrect Handling of totalRewards

In the distributeRewards function, totalRewards is updated with the newly distributed rewards without checking for underflows or ensuring that toDistribute is non-negative. This could lead to unintended consequences if the logic of balanceOf or other parts of the contract is flawed.

  • Ensure that toDistribute is correctly calculated and non-negative.

  • Consider using SafeMath for arithmetic operations, especially if you're working with older versions of Solidity.

3. Event Emission

The emit DistributeRewards event is emitted after the state is modified, which is generally acceptable. However, it may be beneficial to include more context in events, such as balance before distribution, to facilitate easier off-chain tracking.

4. Lack of Access Control

The distributeRewards function is public, which means anyone can call it. This can lead to abuse if users can continuously trigger distributions without any checks.

  • Consider adding access control (e.g., only allowing the controller or specific users to call this function).

5. withdrawableRewards Logic

The withdrawableRewards function directly calls the getUnderlyingByWrapped function, which could potentially lead to inaccurate reward calculations if there are discrepancies in the wrapped and unwrapped token values.

  • Ensure that the getUnderlyingByWrapped function is reliable and accounts for any edge cases, such as rounding errors or discrepancies in token valuations.

6. Overflow and Underflow Vulnerabilities

In Solidity 0.8.x and above, overflow and underflow are automatically checked, but in cases where you're using external libraries or handling user inputs, you may want to explicitly check for these.

7. updateReward Logic

The logic in updateReward does not handle scenarios where the withdrawableRewardsWrapped could be less than userRewards[_account], potentially leading to negative balances.

  • Implement checks to ensure that newRewards does not cause inconsistencies.

8. Missing Input Validation

Input parameters (such as addresses in the constructor) are not validated. If invalid addresses are passed, it could lead to undesired behavior.

  • Add checks to validate addresses and ensure they are not zero addresses.

9. Token Transfer Failure

Although SafeERC20 handles the potential failure of token transfers, it’s still good practice to check if the token.transfer and wsdToken.unwrap calls succeed.

  • Consider wrapping these calls in require statements to ensure success.

Example of Improvements

Here's an example of how you might refactor the _withdraw function to mitigate some of these issues:

function _withdraw(address _account) internal override {
uint256 toWithdraw = withdrawableRewardsWrapped(_account);
require(toWithdraw > 0, "No rewards to withdraw");
// Update state variables before external calls
userRewards[_account] -= toWithdraw;
totalRewards -= toWithdraw;
uint256 toWithdrawUnwrapped = wsdToken.getUnderlyingByWrapped(toWithdraw);
wsdToken.unwrap(toWithdraw); // External call
token.safeTransfer(_account, toWithdrawUnwrapped); // External call
emit Withdraw(_account, toWithdrawUnwrapped);
}

Conclusion

While the code is mostly sound, addressing the points above can help enhance the security and reliability of the contract. Always conduct thorough testing, consider using formal verification methods, and potentially engage in security audits for critical contracts handling user funds.

Updates

Lead Judging Commences

inallhonesty Lead Judge 12 months ago
Submission Judgement Published
Invalidated
Reason: Lack of quality

Support

FAQs

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