Loss of tokens because target address can be set to zero

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-10-ens/blob/main/contracts/ERC20MultiDelegate.sol#L124-L137 https://github.com/code-423n4/2023-10-ens/blob/main/contracts/ERC20MultiDelegate.sol#L155-L161

Vulnerability details

Impact

Due to the possibility of target addresses being zero, tokens transferred to zero address will be lost forever, leading to loss of funds for the users of the protocol and that tokens being out of circulation forever (burned).

Proof of Concept

Users can intentionally or unintentionally (which is the bigger problem) put zero addresses to an array of target addresses inside of the delegateMulti function. Then _processDelegation or createProxyDelegatorAndTransfer functions will transfer tokens to the zero address without any check, resulting in the loss of tokens. A simple target address check inside of those functions will create a much better user experience and prevent the massive burn of tokens.

https://github.com/code-423n4/2023-10-ens/blob/main/contracts/ERC20MultiDelegate.sol#L124-L137

https://github.com/code-423n4/2023-10-ens/blob/main/contracts/ERC20MultiDelegate.sol#L155-L161

Tools Used

Manual analysis

Recommended Mitigation Steps

Add zero address checks for the target address at the start of _processDelegation and createProxyDelegatorAndTransfer functions. If the given target address happens to be zero, skip the rest of the function code.

Assessed type

Token-Transfer

[c4-pre-sort]

141345 marked the issue as low quality report

[141345]

tokens transferred to zero address

not transfer to addr(0), but to the proxyAddress of addr(0)

[c4-judge]

hansfriese marked the issue as unsatisfactory: Invalid