c4-submissions commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-09-delegate/blob/a6dbac8068760ee4fc5bababb57e3fe79e5eeb2e/src/DelegateToken.sol#L310-L314 https://github.com/code-423n4/2023-09-delegate/blob/a6dbac8068760ee4fc5bababb57e3fe79e5eeb2e/src/DelegateToken.sol#L370-L375

Vulnerability details

Impact

High, this will likely happen because protocol plans to work with any/all ERC20 tokens

Summary

Protocol assumes for erc20 tokens that the 'amount' transferred is the same 'amount' that will be deposited in the escrow contract when creating the delegate token, this will not be the case with fee-on-transfer tokens.

When creating a delegate token for erc20 tokens and transferring the tokens to the escrow contract, the protocol will face issues with fee-on-transfer tokens like users not being able to withdraw their tokens at all.

Proof of Concept

Alice wants to delegate rights of 100 tokens of token 'X' from cold wallet to hot wallet or to other people, token 'X' is a fee-on-transfer token and gets transferred to the escrow contract.

100 'X' tokens get transferred but only 90 get deposited, but protocol records her deposit as 100 tokens

create() function erc20 section:

delegateToken.sol  
    } else if (delegateInfo.tokenType == IDelegateRegistry.DelegationType.ERC20) {
        StorageHelpers.writeUnderlyingAmount(delegateTokenInfo, delegateTokenId, delegateInfo.amount);
        newRegistryHash = RegistryHashes.erc20Hash(address(this), delegateInfo.rights, delegateInfo.delegateHolder, delegateInfo.tokenContract);
        StorageHelpers.writeRegistryHash(delegateTokenInfo, delegateTokenId, newRegistryHash);
        RegistryHelpers.incrementERC20(delegateRegistry, newRegistryHash, delegateInfo);

If Alice tries to withraw tokens upon expiry of delegation rights, her tokens will be stuck in the contract and withdraw function will revert because the balance is less than the amount they are trying to withdraw.

withdraw() function code snippet of erc20 section:

delegateToken.sol  
        } else if (delegationType == IDelegateRegistry.DelegationType.ERC20) {
            uint256 erc20UnderlyingAmount = StorageHelpers.readUnderlyingAmount(delegateTokenInfo, delegateTokenId);
            StorageHelpers.writeUnderlyingAmount(delegateTokenInfo, delegateTokenId, 0); // Deletes amount
            RegistryHelpers.decrementERC20(delegateRegistry, registryHash, delegateTokenHolder, underlyingContract, erc20UnderlyingAmount, underlyingRights);
            StorageHelpers.burnPrincipal(principalToken, principalBurnAuthorization, delegateTokenId);
            SafeERC20.safeTransfer(IERC20(underlyingContract), msg.sender, erc20UnderlyingAmount);

Tools Used

Manual review

Recommended Mitigation Steps

Add variable to record balances of tokens 'X' in the contract before and after transfer and record the difference as user's deposit. Other possible measure is to not allow use of fee-on-transfer tokens in protocol.

Assessed type

Token-Transfer

GalloDaSballo commented 12 months ago

OOS from Bot

c4-judge commented 12 months ago

GalloDaSballo marked the issue as unsatisfactory: Out of scope

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

Protocol is incapable of supporting fee-on-transfer tokens #249