hats-finance / Euro-Dollar-0xa4ccd3b6daa763f729ad59eae75f9cbff7baf2cd

Audit competition repository for Euro-Dollar (0xa4ccd3b6daa763f729ad59eae75f9cbff7baf2cd)
https://hats.finance
MIT License
3 stars 2 forks source link

USDE blacklisted user can bypass blacklist by calling InvestToken::deposit #71

Open hats-bug-reporter[bot] opened 2 weeks ago

hats-bug-reporter[bot] commented 2 weeks ago

Github username: -- Twitter username: -- Submission hash (on-chain): 0xf0079c5ee732b3c06e6114d1c85d449d2e7a2b6bdb166573431b48978a1b7fe8 Severity: high

Description: Description\ An USDE blacklisted user can bypass USDE blacklisting mechanism calling InvestToken::deposit setting as receiver a whitelisted address.

Severity: High cause a blacklisted user with a USDE positive balance can bypass blacklist leading USDE blacklisting mechanism useless.

Attack Scenario\ If an address A on validator is blacklisted ie

    Validator::accountStatus[A] = Status.BLACKLISTED;

Then A cant mint and transfer tokens to it, cause USDE::_update function is overriden to:

function _update(address from, address to, uint256 amount) internal override(ERC20*) {
    require(validator.isValid(from, to), "account blocked");
    //...
}

However a blacklisted user (A) with USDE positive balance (A_BALANCE), can bypass USDE blacklist by calling InvestToken::deposit(A_BALANCE, R) with a whitelisted address (R) as a receiver.

    function deposit(uint256 assets, address receiver) public returns (uint256 shares) {
        shares = convertToShares(assets);
=>[1]        usde.burn(msg.sender, assets);
=>[2]       _mint(receiver, shares);
        emit Deposit(msg.sender, receiver, assets, shares);
    }

This will work because in:

[1] InvestToken::deposit(A_BALANCE, R) call flow will be ->

    USDE::burn(A, A_BALANCE)
        USDE::_burn(A, A_BALANCE)
            USDE::_update(A, address(0), A_BALANCE)

Where USDE::_update is defined as:

function _update(address from, address to, uint256 amount) internal override(ERC20Upgradeable, ERC20PausableUpgradeable) {
    require(validator.isValid(from, to), "account blocked");
}

ie USDE::_update will call validator.isValid(A, 0x0):

    function isValid(address from, address to) external view returns (bool valid) {
        return accountStatus[from] == Status.BLACKLISTED ? to == address(0x0) : accountStatus[to] != Status.BLACKLISTED;
    }

Because A is blacklisted it checks if to == address(0), because it is, (its a burn call) then validator::isValid will return true.

So, in [2] InvestToken::deposit(A_BALANCE, R) flow call will be

    InvestToken::_mint(R, shares)
        InvestToken::_update(address(0), R, shares)

where InvestToken::_update is defined as:

function _update(address from, address to, uint256 amount) internal override(ERC20Upgradeable, ERC20PausableUpgradeable) {
    //...
    require(validator.isValidStrict(from, to), "account blocked");
    //...
}

So it will call validator.isValidStrict(address(0), R), where validator::isValidStrict is defined as:

    function isValidStrict(address from, address to) external view returns (bool valid) {
        return  to == address(0x0)
            || (
                accountStatus[to] == Status.WHITELISTED
                    && (from == address(0x0) || accountStatus[from] == Status.WHITELISTED)
            );
    }

Because R is whitelisted and from == address(0) it will return true.

So InvestToken::deposit [1] returns true, and [2] returns true. Also, observe InvestToken::deposit doesnt have any access control modifier so it can be called by anyone at anytime, so even if a user is blacklisted it can move USDE by calling InvestToken::deposit

Attachments

  1. Proof of Concept (PoC) File

  2. Revised Code File (Optional)

AndreiMVP commented 2 weeks ago

Duplicate of https://github.com/hats-finance/Euro-Dollar-0xa4ccd3b6daa763f729ad59eae75f9cbff7baf2cd/issues/58