Lines of code

https://github.com/delegatexyz/delegate-registry/blob/6d1254de793ccc40134f9bec0b7cb3d9c3632bc1/src/DelegateRegistry.sol#L44

Vulnerability details

Description

The delegateAll function does not correctly handle the case where the rights parameter is empty. In this case, the function will attempt to write an empty string to the Storage.POSITIONS_RIGHTS storage location. This behavior could lead to unexpected issues and invalid data in the smart contract storage.

  if (enable) {
if (loadedFrom == Storage.DELEGATION_EMPTY) {
    _pushDelegationHashes(msg.sender, to, hash);
    _writeDelegationAddresses(location, msg.sender, to, address(0));
    if (rights != "") _writeDelegation(location, Storage.POSITIONS_RIGHTS, rights);
}
// ... other code

}

the condition if (rights != "") should prevent writing an empty string to storage, but it's not a robust way to handle this case. An empty string is not the same as no value at all, and writing an empty string may have unintended consequences or waste storage space.

Impact

it could allow an attacker to bypass the intended security checks of the contract. This could lead to the loss of funds or other malicious activity

Proof of Concept:

// Assume Storage.POSITIONS_RIGHTS initially contains a non-empty string.

// Calling delegateAll with an empty 'rights' parameter. delegateAll(addressToDelegateTo, "", true);

Recommendation

To handle this correctly, we should explicitly check whether rights is not empty and only then proceed with writing to storage. Here's a corrected version of that part of the code:

 if (enable) {
     if (loadedFrom == Storage.DELEGATION_EMPTY && bytes(rights).length > 0) {
         _pushDelegationHashes(msg.sender, to, hash);
         _writeDelegationAddresses(location, msg.sender, to, address(0));
         _writeDelegation(location, Storage.POSITIONS_RIGHTS, rights);
     }
     // ... 
 }

By using bytes(rights).length > 0 instead of rights != "", you ensure that you only write to storage when rights is not empty, which is a more robust and precise check.

Assessed type

Invalid Validation

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

Incorrect Handling of Empty rights Parameter in delegateAll Function #247