Assessment reports>Chateau>Critical findings>Potential DOS during ,swap
Category: Business Logic

Potential DOS during swap

Critical Severity
Critical Impact
High Likelihood

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.

Zellic © 2024Back to top ↑