sherlock-audit / 2024-05-kwenta-x-perennial-integration-update-judging

5 stars 3 forks source link

stackbuster23 - Unbounded loop in the _invoke function can lead to denial of service #32

Closed sherlock-admin2 closed 5 months ago

sherlock-admin2 commented 5 months ago

stackbuster23

medium

Unbounded loop in the _invoke function can lead to denial of service

Summary

Looping over an array of unbounded size can lead to denial of service.

Vulnerability Detail

In the MultiInvoker.sol contract the function invoke(address account, Invocation[] calldata invocations) external, which is meant to perform invocations for account, calls function _invoke(address account, Invocation[] calldata invocations) private which Performs a batch of invocations for an account. To carry out this task the function loops through an unbounded array of type struct Invocation. However, If the length of the array of massive size such that the batch invocations of an account (which does involve different kinds of logic in the conditional statements) causes the block to reach its gas limit without the function been able to fully perform its set out objectives.

Impact

This could lead to Ethereum-imposed gas constraint, making the entire transaction to reverts and users unable to interact with the contract.

Code Snippet

https://github.com/sherlock-audit/2024-05-kwenta-x-perennial-integration-update/blob/8e82cd7de2b8d338cd860f670f50c1a96c397bc6/perennial-v2/packages/perennial-extensions/contracts/MultiInvoker.sol#L133-L135
https://github.com/sherlock-audit/2024-05-kwenta-x-perennial-integration-update/blob/8e82cd7de2b8d338cd860f670f50c1a96c397bc6/perennial-v2/packages/perennial-extensions/contracts/MultiInvoker.sol#L140-L145

Tool used

Manual Review

Recommendation

It will be ideal to determine the lenght of the array that the protocol would want to batch process at a given time without rendering the _invoke function’s operation too gas intensive; then, use that number as the upper limit on the number of data that would be processed by the _invoke function.

sherlock-admin2 commented 5 months ago

3 comment(s) were left on this issue during the judging contest.

z3s commented:

Invalid; Denial-of-Service issues require proof of concept (POC), and this issue does not result in the locking of funds or the unavailability of functions. Users can utilize shorter arrays.

takarez commented:

but they are not being saved.

FSchmoede commented:

Invalid, as this would only be an issue for the user himself/herself, thus not affecting anyone else.