ERC20 tokens with blocklist and ERC20 pausable tokens may stuck in contract

[c4-submissions]

Lines of code

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

Vulnerability details

Impact

Some tokens (e.g. USDC) have a contract level admin controlled blocklist. If admin sets address into a blocklist, the transfer from and to that address is forbidden. Other tokens can be paused by admin (e.g BNB). When token is paused, it cannot be transferred.

User may not be able to call withdraw on such tokens, thus tokens will stuck in a contract.

Proof of Concept

File: src/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);

When erc20UnderlyingAmount is paused, or contract's address appears on the blocklist - safeTransfer will always fail - thus withdrawing and burning principal token will not be possible.

Tools Used

Manual code review

Recommended Mitigation Steps

Make sure, that tokens with blocklist or pausable tokens are forbidden in the contract. Otherwise, they may stuck in the protocol.

Assessed type

ERC20

[c4-judge]

GalloDaSballo changed the severity to QA (Quality Assurance)

[GalloDaSballo]

172 L

170 L

169 L

168 R

166 L

4L 1R

[c4-judge]

GalloDaSballo marked the issue as grade-b