meterio / sys-contracts

0 stars 0 forks source link

[METER-LSD-07] Lack of Blacklist Verification for Deposits and Withdrawals in stMTRG #7

Open kalos-security opened 1 year ago

kalos-security commented 1 year ago

[METER-LSD-07] Lack of Blacklist Verification for Deposits and Withdrawals in stMTRG

Impact

The blacklist is not working correctly.

Description

The code below shows that the stMTRG Contract establishes a blacklist and performs sender and receiver verification to prevent unauthorized users from transferring assets.

function _transfer(
        address _from,
        address _to,
        uint256 _value
    ) internal override {
        require(_from != address(0), "ERC20: transfer from the zero address");
        require(_to != address(0), "ERC20: transfer to the zero address");

        _beforeTokenTransfer(_from, _to, _value);
        require(
            !_blackList[_from] && !_blackList[_to],
            "ERC20Pausable: account is in black list"
        );

        uint256 shares = _valueToShare(_value);
        uint256 fromShares = _shares[_from];
        require(fromShares >= shares, "ERC20: transfer amount exceeds balance");
        unchecked {
            _shares[_from] = fromShares - shares;
            _shares[_to] += shares;
        }

        emit Transfer(_from, _to, _value);

        _afterTokenTransfer(_from, _to, _value);
    }

Nevertheless, it is worth noting that the withdraw function does not validate the presence of the withdrawer in the blacklist. Consequently, if one were to employ the withdraw function to exchange stMTRG tokens for MTRG tokens and subsequently make a deposit using a different address, the effectiveness of the blacklist would be disabled

function _withdraw(
        address account,
        uint256 amount,
        address recipient
    ) private {
        uint256 burnShares = _valueToShare(amount);
        uint256 accountShares = _shares[account];
        require(
            accountShares >= burnShares,
            "ERC20: burn amount exceeds balance"
        );
        unchecked {
            _shares[account] = accountShares - burnShares;
            _totalShares -= burnShares.wadToRay();
        }
        emit Transfer(account, address(0), amount);
        scriptEngine.bucketWithdraw(bucketID, amount, recipient);
    }

    function withdraw(uint256 amount, address recipient) public whenNotPaused {
        _withdraw(msg.sender, amount, recipient);
    }

    function exit(address recipient) public whenNotPaused {
        address account = msg.sender;
        uint256 accountShares = _shares[account];
        uint256 amount = balanceOf(account);

        unchecked {
            _shares[account] = 0;
            _totalShares -= accountShares.wadToRay();
        }

        emit Transfer(account, address(0), amount);
        scriptEngine.bucketWithdraw(bucketID, amount, recipient);
    }

Recommendations

Add blacklist functionality to the withdraw functions as well.