Lines of code

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

Vulnerability details

Impact

If the current amount stored in the delegation data is near the maximum value for a uint256, adding a large amount to it could potentially cause an overflow, resulting in an incorrect and unexpected value.

Proof of Concept

The delegateERC1155 function in the contract handles ERC1155 token delegation. It calculates a unique hash and updates delegation-related data. There are potential vulnerabilities related to integer overflow/underflow and consistency in this function:

Potential Overflow/Underflow: The function directly adds the amount parameter to storage without checking for potential overflows or underflows. This can lead to unexpected behavior if the amount exceeds the maximum value that a uint256 can represent or goes negative. It is recommended to use a safe math library like OpenZeppelin's SafeMath to handle these calculations safely.

Consistency and Correctness: The function lacks consistency checks to ensure that the delegation data is updated correctly and that the state transitions are accurate. This can lead to incorrect delegation data or unexpected behaviors in the contract.

Issue

In the delegateERC1155 function, the contract performs several operations based on the amount variable without considering the possibility of overflow or underflow.

if (amount != 0) {
    if (loadedFrom == Storage.DELEGATION_EMPTY) {
        // ...
        _writeDelegation(location, Storage.POSITIONS_TOKEN_ID, tokenId);
        _writeDelegation(location, Storage.POSITIONS_AMOUNT, amount); // Potential overflow/underflow here
        if (rights != "") _writeDelegation(location, Storage.POSITIONS_RIGHTS, rights);
    } else if (loadedFrom == Storage.DELEGATION_REVOKED) {
        _updateFrom(location, msg.sender);
        _writeDelegation(location, Storage.POSITIONS_AMOUNT, amount); // Potential overflow/underflow here
    } else if (loadedFrom == msg.sender) {
        _writeDelegation(location, Storage.POSITIONS_AMOUNT, amount); // Potential overflow/underflow here
    }
} else if (loadedFrom == msg.sender) {
    _updateFrom(location, Storage.DELEGATION_REVOKED);
    _writeDelegation(location, Storage.POSITIONS_AMOUNT, uint256(0)); // Potential overflow/underflow here
}

The potential issue here is related to the manipulation of the amount variable without considering potential overflow or underflow. If the current amount stored in the delegation data is near the maximum value for a uint256, adding a large amount to it could potentially cause an overflow, resulting in an incorrect and unexpected value. Solidity does not automatically handle overflow/underflow, so it's essential to ensure that these cases are appropriately checked.

Potential Exploit Scenario

To illustrate the potential issue, consider a scenario where the current amount stored in the delegation data is already at the maximum value for a uint256, which is 2^256 - 1 (i.e., all bits set to 1). If you add any positive value to this maximum value without checking for overflow, it will wrap around to 0 or another unintended value.

For example, if the current stored amount is:

currentAmount = 115792089237316195423570985008687907853269984665640564039457584007913129639935

And you attempt to add a positive amount like:

amountToAdd = 100

Without proper overflow checking, the result would be:

result = currentAmount + amountToAdd = 100

This is clearly incorrect, as it should have resulted in an overflow and wrapped around to a much smaller value.

Tools Used

Manual Review

Recommended Mitigation Steps

Input Validation: Implement input validation checks to ensure that amount and tokenId are within reasonable and safe ranges. This helps prevent integer overflow and underflow issues.
Use SafeMath: Utilize SafeMath or similar libraries for arithmetic operations to prevent overflows and underflows when updating delegation data.
Consistency Checks: Add consistency checks and validation logic to ensure that delegation data is updated correctly and that state transitions accurately represent token delegations.

Assessed type

Under/Overflow

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

Potential Overflow/Underflow and Consistency Issues in `delegateERC1155` Function. #225