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

2 stars 1 forks source link

Delegation events are emitted even when the state has not changed #349

Closed c4-submissions closed 1 year ago

c4-submissions commented 1 year ago

Lines of code

https://github.com/delegatexyz/delegate-registry/blob/6d1254de793ccc40134f9bec0b7cb3d9c3632bc1/src/DelegateRegistry.sol#L59 https://github.com/delegatexyz/delegate-registry/blob/6d1254de793ccc40134f9bec0b7cb3d9c3632bc1/src/DelegateRegistry.sol#L78 https://github.com/delegatexyz/delegate-registry/blob/6d1254de793ccc40134f9bec0b7cb3d9c3632bc1/src/DelegateRegistry.sol#L98 https://github.com/delegatexyz/delegate-registry/blob/6d1254de793ccc40134f9bec0b7cb3d9c3632bc1/src/DelegateRegistry.sol#L122 https://github.com/delegatexyz/delegate-registry/blob/6d1254de793ccc40134f9bec0b7cb3d9c3632bc1/src/DelegateRegistry.sol#L147

Vulnerability details

Impact

The DelegationRegistry contract on adding new delegation emits corresponding event depending on the type of the delegation. However, the logic of the delegations functions always emits event at the end of the execution even when the state has not actually changed. Example of such a situation would be a case where the delegation already exists or has been revoked and triggering delegation functions would cause the same event being emitted without actually updating anything. This might lead to the confusion of off-chain applications that are monitoring the state of the protocol and delegations.

Proof of Concept

Event DelegateAll emitted at the end of delegateAll function: https://github.com/delegatexyz/delegate-registry/blob/6d1254de793ccc40134f9bec0b7cb3d9c3632bc1/src/DelegateRegistry.sol#L59 Event DelegateContract emitted at the end of delegateContract function: https://github.com/delegatexyz/delegate-registry/blob/6d1254de793ccc40134f9bec0b7cb3d9c3632bc1/src/DelegateRegistry.sol#L78 Event DelegateERC721 emitted at the end of delegateERC721 function: https://github.com/delegatexyz/delegate-registry/blob/6d1254de793ccc40134f9bec0b7cb3d9c3632bc1/src/DelegateRegistry.sol#L98 Event DelegateERC20 emitted at the end of delegateERC20 function: https://github.com/delegatexyz/delegate-registry/blob/6d1254de793ccc40134f9bec0b7cb3d9c3632bc1/src/DelegateRegistry.sol#L122 Event DelegateERC1155 emitted at the end of delegateERC1155 function: https://github.com/delegatexyz/delegate-registry/blob/6d1254de793ccc40134f9bec0b7cb3d9c3632bc1/src/DelegateRegistry.sol#L147

Tools Used

Manual Review

Recommended Mitigation Steps

It is recommended to correct the logic of delegation functions in a way that delegations events will be only emitted in case the state of the delegation has changed.

Assessed type

Other

c4-judge commented 1 year ago

GalloDaSballo marked the issue as duplicate of #347

c4-judge commented 12 months ago

GalloDaSballo changed the severity to QA (Quality Assurance)

GalloDaSballo commented 12 months ago

NC

GalloDaSballo commented 11 months ago

1L 1NC

c4-judge commented 11 months ago

GalloDaSballo marked the issue as grade-c