Liquid Staking

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

contracts/core/test/WrappedSDTokenMock.sol

Your Solidity code for the WrappedSDTokenMock contract looks fairly straightforward, but there are a few potential vulnerabilities and areas for improvement you should consider:

1. Multiplier Manipulation

  • Issue: The setMultiplier function allows anyone to change the multiplier, which could lead to unexpected behavior. If an attacker can manipulate this value, they could effectively alter the conversion rates, leading to significant financial losses for users.

  • Recommendation: Consider restricting access to the setMultiplier function to only the contract owner or a designated role using an access control mechanism, such as OpenZeppelin's Ownable or AccessControl.

2. ERC677 Transfer Handling

  • Issue: The onTokenTransfer function does not validate the _value parameter against the actual amount that was sent. If the transfer was to a contract that does not implement onTokenTransfer correctly, it could lead to unexpected results.

  • Recommendation: Ensure that the amount transferred matches what is expected by implementing a mechanism to verify the transfer or handle potential errors appropriately.

3. Integer Division

  • Issue: The getWrappedByUnderlying function uses integer division. If the _amount is less than the multiplier, it will result in a wrapped amount of zero, which may not be the intended behavior.

  • Recommendation: Implement proper checks or adjust the calculation to ensure that users are aware of the minimum amounts needed for conversions.

4. Burning Before Transfer

  • Issue: In the unwrap function, tokens are burned before the transfer is executed. If the transfer fails for any reason (like insufficient balance), the tokens would already be burned, leading to a loss of tokens without successful unwrapping.

  • Recommendation: Perform the transfer before burning the tokens, or implement a safe transfer pattern to revert the operation if the transfer fails.

5. Missing Events

  • Issue: There are no events emitted for critical state changes such as setMultiplier, minting in onTokenTransfer, and unwrapping in unwrap. This makes it harder to track what happens on-chain and can hinder debugging or off-chain analysis.

  • Recommendation: Emit appropriate events for all significant state changes to enhance transparency and trackability.

6. Potential Reentrancy Risk

  • Issue: The unwrap function is susceptible to reentrancy attacks if an attacker is able to call unwrap recursively while transferring tokens back. This could lead to unintended behavior or loss of funds.

  • Recommendation: Implement a reentrancy guard using a mutex or consider following the checks-effects-interactions pattern by performing state changes before making external calls.

Suggested Code Modifications

Here is a brief example of how you might implement some of these recommendations:

import "@openzeppelin/contracts/access/Ownable.sol";
contract WrappedSDTokenMock is ERC677, Ownable {
...
function setMultiplier(uint256 _multiplier) external onlyOwner {
multiplier = _multiplier;
}
function unwrap(uint256 _amount) external {
require(_amount > 0, "Amount must be > 0");
uint256 unwrappedAmount = getUnderlyingByWrapped(_amount);
// Change the order to prevent potential issues
_burn(msg.sender, _amount);
sdToken.transfer(msg.sender, unwrappedAmount);
emit Unwrapped(msg.sender, _amount, unwrappedAmount); // Emit an event
}
event Unwrapped(address indexed user, uint256 amountWrapped, uint256 amountUnwrapped);
}

Summary

By addressing these points, you can significantly improve the security and reliability of your contract. Always ensure to thoroughly test your code and consider external audits, especially for contracts that manage significant value or user funds.

Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Lack of quality

Support

FAQs

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