Potential DOS during swap
Description
The swap
function iterates through the entire array of issues
. These issues
are created upon performing a stake()
operation, and they essentially represent a user deposit.
Because there is no minimal cap as to the amount of tokens to stake()
in the pool, a malicious user could perform stake()
with one unit of tokens multiple times, which would essentially clog the issues
array with a lot of entries, severely impairing the for loop performed in the swap
function:
for (uint i = indexEnd; i > indexStar; i--) {
Issue storage issueInfo = issues[i];
if (issueInfo.isStaking) {
if (amountB > 0) {
if (amountB >= issueInfo.issueAmount) {
uint amountA = (issueInfo.issueAmount * 10000) / rate;
amountB -= issueInfo.issueAmount;
redeemToekn.safeTransfer(issueInfo.user, amountA);
issueInfo.isStaking = false;
} else {
uint amountA = (amountB * 10000) / rate;
redeemToekn.safeTransfer(issueInfo.user, amountA);
issueInfo.issueAmount -= amountB;
amountB = 0;
}
}
}
}
Impact
The lack of adequate checks in stake()
could potentially lead to a denial of service (DOS) for the StakingPool contract, as a malicious user could clog the system with a lot of small stake
operations, which would make the swap
function perform a lot of unnecessary iterations, eventually reaching the gas limit and rendering the pool unusable.
Recommendations
We recommend implementing a minimum cap for the amount of tokens that can be staked in the pool. This would limit the amount of issues
that could be created and would prevent a potential DOS attack. Additionally, we recommend reevaluating the swap
function to extract the necessary liquidity in various ways, such as using a more efficient algorithm or by using a more dynamic approach to determining which issues
should be considered for the swap.
On top of the aforementioned mitigation directions, we recommend implementing a break condition should amountB
be 0 to prevent unnecessary iterations.
for (uint i = indexEnd; i > indexStar; i--) {
Issue storage issueInfo = issues[i];
if (issueInfo.isStaking) {
if (amountB > 0) {
// ...
+ } else {
+ break;
+ }
}
}
Remediation
This issue has been acknowledged by Chateau Capital, and a fix was implemented in commit 8184d6c5↗.