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

2 stars 1 forks source link

Protocol wont work with tokens that can block or prevent transfers e.g Pausable, Blacklist, Blocking etc #218

Closed c4-submissions closed 1 year ago

c4-submissions commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-09-delegate/blob/a6dbac8068760ee4fc5bababb57e3fe79e5eeb2e/src/libraries/DelegateTokenTransferHelpers.sol#L58 https://github.com/code-423n4/2023-09-delegate/blob/a6dbac8068760ee4fc5bababb57e3fe79e5eeb2e/src/DelegateToken.sol#L402 https://github.com/code-423n4/2023-09-delegate/blob/a6dbac8068760ee4fc5bababb57e3fe79e5eeb2e/src/libraries/DelegateTokenTransferHelpers.sol#L21

Vulnerability details

Impact

There are various tokens and token standards that can result in transfers being stopped, blocked, blacklisted, paused or disallowed. This entails protocols may function well with these tokens up until a time when any of above measures activated leading to inability to perform transfers into and out of the protocol

Proof of Concept

Tokens such as ERC20Pausable, Pausable Tokens like WBTC, ERC1400, Polymath like tokens, USDC has Circle which can block certain addresses; it implies all instances mentioned in the few above links and many others not mentioned with token transfers where tokens can be in described category e.g USDC results in those function parts not being able to perform transfers if block activated on the tokens.

e.g below example DelegateTokenTransferHelpers.sol line 58

 function pullERC20AfterCheck(address underlyingContract, uint256 pullAmount) internal {
        SafeERC20.safeTransferFrom(IERC20(underlyingContract), msg.sender, address(this), pullAmount);
    }

What's worse is tokens with blacklisting capabilities may block the various contract addresses of protocol, it admins, governance contracts etc which renders them incapable to send and receive these tokens to function fully.

Tools Used

Manual Analysis

Recommended Mitigation Steps

It is recommended such tokens be disallowed from being used with the protocol Consider a whitelist of allowed tokens that excludes such tokens Consider a policy to use the pause and unpause functionality if the tokens active their blocking capabilities in instances that are detrimental to the protocol

Assessed type

Token-Transfer

c4-judge commented 1 year ago

GalloDaSballo changed the severity to QA (Quality Assurance)

0xfoobar commented 1 year ago

Of course people can include arbitrary logic in their tokens, including bricking them. This issue is irrelevant, it applies to Uniswap V2 as much as it does to us.

c4-sponsor commented 1 year ago

0xfoobar (sponsor) disputed