Malicious users could grief innocent users into receiving fewer points
Summary
Malicious users could grief innocent users into receiving fewer points.
Vulnerability Detail
Malicious users could grief innocent users by repeatedly depositing or opening leverage of the smallest possible amount to trigger an update of the mintRate.lastMintTimestamp state variable to the current timestamp. The minimum deposit is 1e6 wei and costs 0.000000003523 USD, which is almost nothing for malicious users to carry out the attack. There is no minimum size for opening a leverage position. They could use multiple accounts to trigger an update every block, Given that the gas fee in the Base chain is extremely cheap, it is practical to carry out this attack.
Let block.timestamp - mintRate.lastMintTimestamp in the formula be $\Delta T$. Since mintRate.lastMintTimestamp is updated frequently or updated every block, it is very close to the current block's timestamp (block.timestamp). Thus, $\Delta T$ will be an extremely small value.
Assume the following states:
Let mintRate.decayTime be $D$ and its value is set to 24 hours (86400 seconds)
Let mintRate.lastAccumulatedMint be $lastAccMint$ and its value set to 10000e18
Let mintRate.maxAccumulatedMint be $maxAccMint$ and its value is set to 10000e18.
Following is the formula for the getAccumulatedMint() function:
If $\Delta T$ is always an extremely small value, the decay will always be close to one ($\approx 0.99999..$). This means that there is almost no decay, or the decay has been significantly slowed down, and the value of $accMint$ returned will hover close to the $maxAccMint$.
Since the amount of points available to mint to the users is computed by $maxAccMint - accMint$, only a very small amount (or dust amount of points) will be left to be minted to the users. Due to the griefing attack by malicious users, the innocent users will not be able to receive the points that they would have received if the griefing attack did not occur.
When the points contract is deployed, and the mintRate.maxAccumulatedMint is set to 10000e18, a malicious user could quickly mint all the 10000e18 points and perform the abovementioned attacks to prevent the decay mechanism from reducing the accumulated mint amount. As a result, the malicious user always owns the majority of the points in the protocol, while the rest of the users could only own a very small/dust amount of points, creating unfairness within the protocol.
Impact
Loss of points for the innocent users. In the main audit contest, points are deemed items that contain value as issues related to points are classified as Medium.
File: PointsModule.sol
175: function _mintTo(address to, uint256 amount) internal {
176: // Ignore dust amounts. It avoids potential precision errors on unlock time calculations
177: if (amount < minMintAmount) return;
A similar approach could be taken within the _updateAccumulatedMint function to make such a griefing attack uneconomically for the malicious users. Currently, even if the returned _amountToMint is zero or 1 wei, the code at Line 219 will still proceed to update the mintRate.lastMintTimestamp to the current block's timestamp even when no points are minted to the users, which is incorrect and inconsistent.
File: PointsModule.sol
207: /// @notice Update the accumulated points mint
208: /// @dev It will only update the accumulated mint if it is within the rate limit
209: function _updateAccumulatedMint(uint256 _mintAmount) internal returns (uint256 _amountToMint) {
210: uint256 newAccumulatedMint = _mintAmount + getAccumulatedMint();
211:
212: if (newAccumulatedMint > mintRate.maxAccumulatedMint) {
213: _amountToMint = getAvailableMint();
214: mintRate.lastAccumulatedMint = mintRate.maxAccumulatedMint;
215: } else {
216: _amountToMint = _mintAmount;
217: mintRate.lastAccumulatedMint = newAccumulatedMint;
218: }
219: mintRate.lastMintTimestamp = block.timestamp.toUint64();
220: }
Consider the following changes to make the griefing attack more expensive to be carried out for deterrence.
/// @notice Update the accumulated points mint
/// @dev It will only update the accumulated mint if it is within the rate limit
function _updateAccumulatedMint(uint256 _mintAmount) internal returns (uint256 _amountToMint) {
uint256 newAccumulatedMint = _mintAmount + getAccumulatedMint();
if (newAccumulatedMint > mintRate.maxAccumulatedMint) {
_amountToMint = getAvailableMint();
mintRate.lastAccumulatedMint = mintRate.maxAccumulatedMint;
} else {
_amountToMint = _mintAmount;
mintRate.lastAccumulatedMint = newAccumulatedMint;
}
+ if (amount < minMintAmount) {
mintRate.lastMintTimestamp = block.timestamp.toUint64();
+ }
}
In addition, the current minMintAmount is set to 1e6 during initialization, and there is no way to update this value after initialization. Consider allowing the minMintAmount variable to be updatable by the owner and the value to be set to a sufficiently high value (1e6 is too small) so that it will be expensive for the attacker to mint the minimum amount of points repeatedly. If the users do not meet the minMintAmount, the minting of points should be skipped, and no update to mintRate.lastAccumulatedMint should be made.
xiaoming90
medium
Malicious users could grief innocent users into receiving fewer points
Summary
Malicious users could grief innocent users into receiving fewer points.
Vulnerability Detail
Malicious users could grief innocent users by repeatedly depositing or opening leverage of the smallest possible amount to trigger an update of the
mintRate.lastMintTimestampstate variable to the current timestamp. The minimum deposit is 1e6 wei and costs 0.000000003523 USD, which is almost nothing for malicious users to carry out the attack. There is no minimum size for opening a leverage position. They could use multiple accounts to trigger an update every block, Given that the gas fee in the Base chain is extremely cheap, it is practical to carry out this attack.Let
block.timestamp - mintRate.lastMintTimestampin the formula be $\Delta T$. SincemintRate.lastMintTimestampis updated frequently or updated every block, it is very close to the current block's timestamp (block.timestamp). Thus, $\Delta T$ will be an extremely small value.Assume the following states:
mintRate.decayTimebe $D$ and its value is set to 24 hours (86400 seconds)mintRate.lastAccumulatedMintbe $lastAccMint$ and its value set to 10000e18mintRate.maxAccumulatedMintbe $maxAccMint$ and its value is set to 10000e18.Following is the formula for the
getAccumulatedMint()function:$$ \begin{align} decay = \frac{D - min(D, \Delta T)}{D} \ accMint = lastAccMint \times decay \end{align} $$
If $\Delta T$ is always an extremely small value, the decay will always be close to one ($\approx 0.99999..$). This means that there is almost no decay, or the decay has been significantly slowed down, and the value of $accMint$ returned will hover close to the $maxAccMint$.
https://github.com/sherlock-audit/2024-03-flat-money-fix-review-contest/blob/main/flatcoin-v1/src/PointsModule.sol#L138
Since the amount of points available to mint to the users is computed by $maxAccMint - accMint$, only a very small amount (or dust amount of points) will be left to be minted to the users. Due to the griefing attack by malicious users, the innocent users will not be able to receive the points that they would have received if the griefing attack did not occur.
When the points contract is deployed, and the
mintRate.maxAccumulatedMintis set to 10000e18, a malicious user could quickly mint all the 10000e18 points and perform the abovementioned attacks to prevent the decay mechanism from reducing the accumulated mint amount. As a result, the malicious user always owns the majority of the points in the protocol, while the rest of the users could only own a very small/dust amount of points, creating unfairness within the protocol.Impact
Loss of points for the innocent users. In the main audit contest, points are deemed items that contain value as issues related to points are classified as Medium.
Code Snippet
https://github.com/sherlock-audit/2024-03-flat-money-fix-review-contest/blob/main/flatcoin-v1/src/PointsModule.sol#L138
Tool used
Manual Review
Recommendation
When the amount of points to be minted to the user is below the
minMintAmount, the_mintTofunction will short-circuit and exit immediately.https://github.com/sherlock-audit/2024-03-flat-money-fix-review-contest/blob/main/flatcoin-v1/src/PointsModule.sol#L175
A similar approach could be taken within the
_updateAccumulatedMintfunction to make such a griefing attack uneconomically for the malicious users. Currently, even if the returned_amountToMintis zero or 1 wei, the code at Line 219 will still proceed to update themintRate.lastMintTimestampto the current block's timestamp even when no points are minted to the users, which is incorrect and inconsistent.https://github.com/sherlock-audit/2024-03-flat-money-fix-review-contest/blob/main/flatcoin-v1/src/PointsModule.sol#L219
Consider the following changes to make the griefing attack more expensive to be carried out for deterrence.
In addition, the current
minMintAmountis set to 1e6 during initialization, and there is no way to update this value after initialization. Consider allowing theminMintAmountvariable to be updatable by the owner and the value to be set to a sufficiently high value (1e6 is too small) so that it will be expensive for the attacker to mint the minimum amount of points repeatedly. If the users do not meet theminMintAmount, the minting of points should be skipped, and no update tomintRate.lastAccumulatedMintshould be made.Duplicate of #10