Liquid Staking
Stakelink
DeFiHardhatOracle
50,000 USDC
View results
Previous Next
Submission Details
Severity: high
Invalid

contracts/core/test/LSTMock.sol

kp8877

The provided Solidity code has several aspects to consider in terms of potential vulnerabilities and improvements. Here are some points to note:

1. Function Accessibility

  • setMultiplierBasisPoints Function: This function is public and allows anyone to change the mulitplierBasisPoints variable. This could lead to manipulation of the multiplier by unauthorized users. You should consider adding access control (e.g., using OpenZeppelin's Ownable or similar pattern) to restrict who can set this multiplier.

2. Spelling Error in Variable Name

  • mulitplierBasisPoints: The variable is consistently misspelled as "mulitplier." This could lead to confusion and is a bad practice in terms of code readability. The correct spelling should be "multiplier."

3. Division by Zero

  • While the provided code doesn't currently risk a division by zero error, if mulitplierBasisPoints were to be set to zero, it would lead to division by zero in the getSharesByStake and getStakeByShares functions. It’s prudent to add checks before performing such divisions.

4. Integer Overflow and Underflow

  • Although Solidity 0.8.x has built-in checks for overflow and underflow, it’s essential to consider the logic within your functions. Ensure that your calculations will not lead to unexpected results, especially when using multipliers.

5. Event Emission

  • State Changes: If setMultiplierBasisPoints modifies the state, it should ideally emit an event to signal the change. This helps with tracking state changes on the blockchain.

6. Reentrancy Considerations

  • Depending on how ERC677 is implemented, ensure that functions in this contract don’t allow for reentrancy attacks. Since it appears to be extending an ERC677 token, review the base contract’s implementation for any reentrancy vulnerabilities.

7. Magic Numbers

  • Using 10000 as a hardcoded value can lead to confusion. It’s a good practice to define such constants in the contract, making it easier to understand their purpose. You could define a constant like:

    uint256 public constant BPS = 10000; // Basis points

8. Input Validation

  • The function setMultiplierBasisPoints does not validate the input value. It would be prudent to check that the new multiplier is within a reasonable range before updating the state.

Suggested Improvements

Here’s a refactored version with some of the concerns addressed:

// SPDX-License-Identifier: GPL-3.0
pragma solidity 0.8.15;
​
import "../tokens/base/ERC677.sol";
import "@openzeppelin/contracts/access/Ownable.sol";
​
contract LSTMock is ERC677, Ownable {
uint256 public multiplierBasisPoints;
uint256 public constant BPS = 10000; // Basis points
​
constructor(
string memory _tokenName,
string memory _tokenSymbol,
uint256 _totalSupply
) ERC677(_tokenName, _tokenSymbol, _totalSupply) {
multiplierBasisPoints = BPS; // Initialize to 100%
}
​
function balanceOf(address _account) public view override returns (uint256) {
return (super.balanceOf(_account) * multiplierBasisPoints) / BPS;
}
​
function getSharesByStake(uint256 _amount) external view returns (uint256) {
require(multiplierBasisPoints > 0, "Multiplier cannot be zero");
return (_amount * BPS) / multiplierBasisPoints;
}
​
function getStakeByShares(uint256 _sharesAmount) external view returns (uint256) {
return (_sharesAmount * multiplierBasisPoints) / BPS;
}
​
function setMultiplierBasisPoints(uint256 _multiplierBasisPoints) external onlyOwner {
require(_multiplierBasisPoints > 0, "Multiplier must be greater than zero");
multiplierBasisPoints = _multiplierBasisPoints;
emit MultiplierUpdated(_multiplierBasisPoints);
}
​
event MultiplierUpdated(uint256 newMultiplier);
}

Summary

  • Ensure proper access control for sensitive state-changing functions.

  • Maintain good coding practices, such as naming conventions and input validation.

  • Incorporate event emissions for better transparency and tracking on the blockchain.

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.