Staking functionality temporary blocking due to lack of address zero check

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-10-ethena/blob/ee67d9b542642c9757a6b826c82d0cae60256509/contracts/StakedUSDe.sol#L106-L127 https://github.com/code-423n4/2023-10-ethena/blob/ee67d9b542642c9757a6b826c82d0cae60256509/contracts/StakedUSDe.sol#L245-L252

Vulnerability details

Impact

Though lack of zero check issue for the addToBlacklist function is already at the automated findings output, I suppose it is necessary to show the importance of this check here. In case of accidental or malicious the BLACKLIST_MANAGER_ROLE behavior the StakedUSDe contract can be blocked by adding address(0) as FULL_RESTRICTED_STAKER_ROLE. All calls to _deposit and _withdraw will be aborted due to the _beforeTokenTransfer hook until address(0) is removed from the blacklist.

Proof of Concept

When the _beforeTokenTransfer hook of the StakedUSDe contract is called from _mint or _burn functions it receives address(0) as parameter: ERC20:

    function _mint(address account, uint256 amount) internal virtual {
        require(account != address(0), "ERC20: mint to the zero address");

        _beforeTokenTransfer(address(0), account, amount);
        ...

    function _burn(address account, uint256 amount) internal virtual {
        require(account != address(0), "ERC20: burn from the zero address");

        _beforeTokenTransfer(account, address(0), amount);
        ...

If address(0) will be added as FULL_RESTRICTED_STAKER_ROLE the _beforeTokenTransfer will be always revert:

  function _beforeTokenTransfer(address from, address to, uint256) internal virtual override {
    if (hasRole(FULL_RESTRICTED_STAKER_ROLE, from) && to != address(0)) {
      revert OperationNotAllowed();
    }
    if (hasRole(FULL_RESTRICTED_STAKER_ROLE, to)) {
      revert OperationNotAllowed();
    }
  }

https://github.com/code-423n4/2023-10-ethena/blob/ee67d9b542642c9757a6b826c82d0cae60256509/contracts/StakedUSDe.sol#L245-L252

So the BLACKLIST_MANAGER_ROLE can easily block the contract. Only BLACKLIST_MANAGER_ROLE can remove address(0) from the blacklist:

  function removeFromBlacklist(address target, bool isFullBlacklisting)
    external
    onlyRole(BLACKLIST_MANAGER_ROLE)
    notOwner(target)
  {

https://github.com/code-423n4/2023-10-ethena/blob/ee67d9b542642c9757a6b826c82d0cae60256509/contracts/StakedUSDe.sol#L120-L124

To fix the problem in case of malicious behavior the DEFAULT_ADMIN_ROLE should remove address from BLACKLIST_MANAGER_ROLE and add another.

Tools Used

Manual review

Recommended Mitigation Steps

Consider adding address zero check at the addToBlacklist function.

Assessed type

DoS

[c4-pre-sort]

raymondfam marked the issue as low quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #85

[c4-judge]

fatherGoose1 marked the issue as unsatisfactory: Invalid