code-423n4 / 2023-01-drips-findings

0 stars 2 forks source link

[M-01] `emitUserMetadata` function may fail due to exceed gas limit #210

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-01-drips/blob/main/src/DripsHub.sol#L613

Vulnerability details

Impact

The function emitUserMetadata in DripsHub may fail due to unbounded loop over userMetadata can be very large due to the user input. However, function could be called only from drivers, it's still public and large array could be passed. And the loop in emitUserMetadata did not have a mechanism to stop, it’s only based on the array length, and may take all the gas limit. If the gas limit is reached, this transaction will fail or revert.

Proof of Concept

for (uint256 i = 0; i < userMetadata.length; i++) {
    UserMetadata calldata metadata = userMetadata[i];
    emit UserMetadataEmitted(userId, metadata.key, metadata.value);
}

Tools Used

Manual audit

Recommended Mitigation Steps

Perform userMetadata length check and revert if necessary.

GalloDaSballo commented 1 year ago

Thinking QA at best, believe the cost is in the hundreds of gas

Screenshot 2023-02-06 at 21 31 21
xmxanuel commented 1 year ago

Technical correct but not an issue. In that case, the gasLimit forms a natural maxLength which is currently more than sufficient for the use-case we have in mind.

Emitting events is very cheap. A user could still emit thousands of events.

CodeSandwich commented 1 year ago

[dispute validity] What Alex and Manuel said.

c4-sponsor commented 1 year ago

CodeSandwich marked the issue as sponsor disputed

c4-judge commented 1 year ago

GalloDaSballo marked the issue as unsatisfactory: Insufficient proof

GalloDaSballo commented 1 year ago

Closing due to: