Lines of code

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

Vulnerability details

Impact

The checkDelegateForERC20 function in the DelegateRegistry contract has a potential issue related to the handling of the amount parameter when interacting with ERC20 tokens. The contract assumes a fixed decimal precision of 18 for all ERC20 tokens, which may not be accurate for tokens with different decimal precisions.

function checkDelegateForERC20(address to, address from, address contract_, bytes32 rights) external view override returns (uint256 amount) {
    if (!_invalidFrom(from)) {
        amount = (_validateFrom(Hashes.allLocation(from, "", to), from) || _validateFrom(Hashes.contractLocation(from, "", to, contract_), from))
            ? type(uint256).max
            : _loadDelegationUint(Hashes.erc20Location(from, "", to, contract_), Storage.POSITIONS_AMOUNT);
        if (!Ops.or(rights == "", amount == type(uint256).max)) {
            uint256 rightsBalance = (_validateFrom(Hashes.allLocation(from, rights, to), from) || _validateFrom(Hashes.contractLocation(from, rights, to, contract_), from))
                ? type(uint256).max
                : _loadDelegationUint(Hashes.erc20Location(from, rights, to, contract_), Storage.POSITIONS_AMOUNT);
            amount = Ops.max(rightsBalance, amount);
        }
    }
    // ...
}

Issue Explanation:

The problem arises because the contract directly retrieves the amount from storage without considering the token's actual decimal precision. This can lead to incorrect calculations and balance reporting when interacting with tokens that have a different number of decimal places.

Impact of the Issue: This issue can have the following impacts:

Incorrect Balance Reporting: When interacting with ERC20 tokens that have decimal precisions different from 18, the contract may report incorrect balances, leading to user confusion.
Incorrect Decision Making: If the contract makes decisions or calculations based on the reported balance, it can lead to unintended actions or errors.
Loss of Tokens: Users may lose tokens or receive incorrect amounts when the contract performs token transfers or calculations based on incorrect balance information.

Proof of Concept

To illustrate the issue, consider the following scenario:

Scenario: Handling Tokens with 2 Decimal Places

Token A has 2 decimal places, and Token B has 18 decimal places.
The checkDelegateForERC20 function is called to check the balance of Token A for a user.
The contract retrieves the amount of Token A without adjusting it for the 2 decimal places, assuming it has 18 decimal places.
If the user has 1 Token A, the contract may incorrectly report the balance as 100 Token A (1 * 10^2) instead of 1 Token A.

Consequences: The consequences of not considering the token's decimal precision can be significant:

Incorrect Balance Reporting: Users may receive incorrect balance information, leading to confusion and potential mistakes in their interactions with the contract.
Incorrect Decision Making: If the contract makes decisions or calculations based on the reported balance, it can lead to unintended actions or errors.
Loss of Tokens: Users may lose tokens or receive incorrect amounts when the contract performs token transfers or calculations based on incorrect balance information.

Tools Used

Manual Review

Recommended Mitigation Steps

Ensure accurate handling of ERC20 tokens with varying decimal precisions, consider the following steps:

Retrieve Token Decimals: Before processing any token balance checks, obtain the token's decimal precision by calling the decimals function on the ERC20 token contract. This will provide you with the correct decimal value for the token.
Adjust the Amount: Divide the retrieved amount by 10^decimals (where decimals is the obtained decimal precision from the token contract) before using it in any calculations or returning it. This ensures that the amount is correctly adjusted to match the token's precision.

uint256 tokenDecimals = ERC20(contract_).decimals(); // Get the token's decimal value

if (!_invalidFrom(from)) {
    amount = (_validateFrom(Hashes.allLocation(from, "", to), from) || _validateFrom(Hashes.contractLocation(from, "", to, contract_), from))
        ? type(uint256).max
        : _loadDelegationUint(Hashes.erc20Location(from, "", to, contract_), Storage.POSITIONS_AMOUNT);
    if (!Ops.or(rights == "", amount == type(uint256).max)) {
        uint256 rightsBalance = (_validateFrom(Hashes.allLocation(from, rights, to), from) || _validateFrom(Hashes.contractLocation(from, rights, to, contract_), from))
            ? type(uint256).max
            : _loadDelegationUint(Hashes.erc20Location(from, rights, to, contract_), Storage.POSITIONS_AMOUNT);
        amount = Ops.max(rightsBalance, amount);
    }
}
// Divide the amount by 10^decimals to adjust it
amount = amount / (10**tokenDecimals);

Assessed type

Decimal

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

Decimal Precision Issue in `checkDelegateForERC20` Function. #230