search

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

2 stars 1 forks source link

DelegateRegistry.checkDelegateForERC20() and checkDelegateForERC1155() will return the maximum amount delegated even if there are multiple matching delegations with different amounts #255

Closed c4-submissions closed 12 months ago

c4-submissions commented 1 year ago

Lines of code

https://github.com/delegatexyz/delegate-registry/blob/6d1254de793ccc40134f9bec0b7cb3d9c3632bc1/src/DelegateRegistry.sol#L210-L226 https://github.com/delegatexyz/delegate-registry/blob/6d1254de793ccc40134f9bec0b7cb3d9c3632bc1/src/DelegateRegistry.sol#L229-L245

Vulnerability details

Impact

It may over-report the total amount delegated in some cases

Proof of Concept

The key functions are checkDelegateForERC20 and checkDelegateForERC1155. These check if a given address from has delegated tokens to address to. The logic checks matching delegations in this order:

  1. A delegation with no specified rights
  2. A delegation for the specific contract_
  3. A delegation for the specific rights It returns the maximum amount from any matching delegation. This means if there are 2 delegations:
  4. Delegation 1: 1,000 tokens delegated with no rights specified
  5. Delegation 2: 500 tokens delegated with specific rights checkDelegateForERC20 will return 1,000 tokens, even though the total delegated is only 1,500.

Tools Used

Manual

Recommended Mitigation Steps

• Return an array with all matching delegation amounts instead of just the maximum • Aggregate the amounts from all matching delegations and return the total

Assessed type

Other

c4-judge commented 12 months ago

GalloDaSballo marked the issue as unsatisfactory: Insufficient proof