QuantAMM

QuantAMM
49,600 OP
View results
Submission Details
Severity: medium
Invalid

Redundant `delete` Operation in `UpliftOnlyExample.afterUpdate` Function Leads to Unnecessary Gas Consumption and Potential Denial of Service

Summary

The afterUpdate function in the UpliftOnlyExample contract contains a redundant delete operation within a loop that reorders elements in the feeDataArray. This loop is executed when an NFT representing a liquidity position is transferred. The unnecessary delete operation consumes extra gas for each iteration of the loop, leading to higher transaction costs for users. In cases where the feeDataArray is large, this inefficiency can potentially cause transactions to exceed the block gas limit, resulting in a denial of service. This issue can be classified as a medium severity vulnerability.

Vulnerability Details

The vulnerability resides in the afterUpdate function of the UpliftOnlyExample contract, specifically within the loop responsible for reordering elements in the feeDataArray array, located at UpliftOnlyExample.sol#L616-L622. When an NFT representing a liquidity position is transferred, this function ensures that the associated FeeData is moved from the old owner's array to the new owner's array within the poolsFeeData mapping, maintaining a First-In-Last-Out (FILO) order.

The loop iterates through a portion of the feeDataArray, shifting elements to fill the gap left by the removed FeeData entry. However, inside the loop, each iteration unnecessarily includes a delete operation on the element that will be overwritten in the subsequent assignment statement. This delete operation serves no functional purpose, as the element is immediately overwritten, but it consumes extra gas.

if (tokenIdIndex != feeDataArrayLength - 1) {
//Reordering the entire array could be expensive but it is the only way to keep true FILO
for (uint i = tokenIdIndex + 1; i < feeDataArrayLength; i++) {
@> delete feeDataArray[i - 1];
feeDataArray[i - 1] = feeDataArray[i];
}
}
delete feeDataArray[feeDataArrayLength - 1];
feeDataArray.pop();

The redundant delete operation leads to increased gas consumption for each transfer of an LP NFT. The impact of this inefficiency becomes more pronounced as the size of the feeDataArray increases, approaching the maximum allowed 100 deposits per user. In such cases, the loop can become excessively expensive, potentially causing transactions to exceed the block gas limit and resulting in a denial of service.

Impact

The redundant delete operation within the afterUpdate function's loop leads to several negative consequences:

  1. Increased Gas Costs: Users transferring LP NFTs will incur unnecessarily higher gas costs due to the extra gas consumed by the redundant delete operation. This impact is directly proportional to the size of the feeDataArray being reordered.

  2. Potential Denial of Service (DoS): When the feeDataArray is large (approaching the maximum limit of 100), the loop's gas consumption can become excessive. This can cause transactions to exceed the block gas limit, effectively leading to a denial of service for NFT transfers. Users would be unable to transfer their LP NFTs, potentially trapping their assets.

  3. Protocol Inefficiency: The vulnerability introduces unnecessary gas consumption into a core function of the protocol, reducing its overall efficiency and potentially making it less attractive to users compared to more gas-optimized alternatives.

  4. Economic Attack Vector (Minor): While not a direct security vulnerability, consistently high gas costs could be exploited in economic attacks. An attacker could potentially manipulate gas prices or create scenarios where the inflated gas cost becomes a factor in arbitrage or other trading strategies, though this would be a complex and likely less profitable attack vector.

The severity of this issue is Medium. While it doesn't directly compromise the security of funds or allow unauthorized actions, the potential for DoS and the unnecessary gas costs for a core function are significant enough to warrant attention and remediation.

Tools Used

Manual Review

Recommendations

The recommended remediation is to remove the unnecessary delete operation from within the loop in the afterUpdate function. This simple change eliminates the redundant gas consumption without affecting the function's logic or the desired FILO ordering.

The corrected code should be as follows:

if (tokenIdIndex != feeDataArrayLength - 1) {
//Reordering the entire array could be expensive but it is the only way to keep true FILO
for (uint i = tokenIdIndex + 1; i < feeDataArrayLength; i++) {
- delete feeDataArray[i - 1];
feeDataArray[i - 1] = feeDataArray[i];
}
}

By removing the line delete feeDataArray[i - 1];, the loop will only perform the necessary element shifting, significantly reducing gas consumption, especially when the feeDataArray is large. This change will mitigate the risk of denial of service and improve the overall efficiency of the protocol.

Updates

Lead Judging Commences

n0kto Lead Judge 11 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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

Give us feedback!