code-423n4 / 2023-09-delegate-findings

2 stars 1 forks source link

DelegateRegistry enumeration functions may break for big arrays of outgoing and incoming delegations #341

Closed c4-submissions closed 10 months ago

c4-submissions commented 10 months ago

Lines of code

https://github.com/delegatexyz/delegate-registry/blob/6d1254de793ccc40134f9bec0b7cb3d9c3632bc1/src/DelegateRegistry.sol#L338-L341 https://github.com/delegatexyz/delegate-registry/blob/6d1254de793ccc40134f9bec0b7cb3d9c3632bc1/src/DelegateRegistry.sol#L386-L406 https://github.com/delegatexyz/delegate-registry/blob/6d1254de793ccc40134f9bec0b7cb3d9c3632bc1/src/DelegateRegistry.sol#L417-L425

Vulnerability details

Impact

The DelegateRegistry contract on every delegation triggers _pushDelegationHashes function to add delegation hash to outgoingDelegationHashes and incomingDelegationHashes arrays of the corresponding from or to addresses within the mapping. Over time the size of the array will keep growing and it might be big enough to break the execution of enumeration functions since they require iteration over the whole array of given outgoing or incoming delegations.

Proof of Concept

Function responsible for increasing the outgoing and incoming delegation hashes arrays: https://github.com/delegatexyz/delegate-registry/blob/6d1254de793ccc40134f9bec0b7cb3d9c3632bc1/src/DelegateRegistry.sol#L338-L341

Enumeration functions that iterates over arrays: https://github.com/delegatexyz/delegate-registry/blob/6d1254de793ccc40134f9bec0b7cb3d9c3632bc1/src/DelegateRegistry.sol#L386-L406 https://github.com/delegatexyz/delegate-registry/blob/6d1254de793ccc40134f9bec0b7cb3d9c3632bc1/src/DelegateRegistry.sol#L417-L425

Tools Used

Manual Review

Recommended Mitigation Steps

It is recommended to add additional parameters such as offset and limit to enumeration functions that will allow retrieving the required range of delegations.

Assessed type

DoS

c4-judge commented 10 months ago

GalloDaSballo marked the issue as duplicate of #99

c4-judge commented 9 months ago

GalloDaSballo changed the severity to QA (Quality Assurance)