re-al-Foundation / rwa-contracts

Core dev environment for the RWA Governance contracts
0 stars 0 forks source link

[RWA-02M] Potentially Incorrect Blacklist Application #48

Closed chasebrownn closed 7 months ago

chasebrownn commented 7 months ago

RWA-02M: Potentially Incorrect Blacklist Application

Type Severity Location
Logical Fault RWAToken.sol:L260, L261

Description:

The referenced statements that apply the isBlacklisted restriction will trigger solely when both the from and the to addresses are not in the isExcludedFromFees entries.

Impact:

The blacklist mechanism of the RWAToken is improperly enforced and permits it to be circumvented by interacting with system contracts.

Example:

/**
 * @notice Transfers an `amount` amount of tokens from `from` to `to`, or alternatively mints (or burns) if `from`.
 * @dev This overrides `_update` from ERC20Upgradeable.`
 *      Unless `from` or `to` is excluded, there will be a tax on the transfer.
 * @param from Address balance decreasing.
 * @param to Address balance increasing.
 * @param amount Amount of tokens being transferred from `from` to `to`.
 */
function _update(address from, address to, uint256 amount) internal override {

    // note: if automatedMarketMakerPairs[from] == true -> BUY
    // note: if automatedMarketMakerPairs[to] == true   -> SELL

    bool excludedAccount = isExcludedFromFees[from] || isExcludedFromFees[to];

    if (!excludedAccount) { // if not whitelisted

        if (isBlacklisted[from]) revert Blacklisted(from);
        if (isBlacklisted[to]) revert Blacklisted(to);

        // if transaction is a buy or sell, take fee.
        if(automatedMarketMakerPairs[from] || automatedMarketMakerPairs[to]) {
            uint256 feeAmount;

            feeAmount = (amount * fee) / 100;       
            amount -= feeAmount;

            require(royaltyHandler != address(0), "RWAToken: No royalty handler assigned");
            super._update(from, royaltyHandler, feeAmount);
        }
    }

    super._update(from, to, amount);
    require(totalSupply() <= MAX_SUPPLY, "RWAToken: max. supply exceeded");
}

Recommendation:

We advise the code to ensure that the blacklists are unconditionally enforced as otherwise, a blacklisted user can circumvent them by f.e. vesting their RWA token to veRWA.

chasebrownn commented 7 months ago

Resolved