TangibleTNFT / ustb

MIT License
0 stars 1 forks source link

[RTU-02M] Inexistent Support of Rebasing Status #2

Open dkuppitz opened 8 months ago

dkuppitz commented 8 months ago

RTU-02M: Inexistent Support of Rebasing Status

Type Severity Location
Logical Fault RebaseTokenUpgradeable.sol:L194

Description:

The overall USTB system does support an optOut system of the rebasing mechanism, however, the actual balance-updating function of RebaseTokenUpgradeable::_update does not properly support this. As a result, the USTB token will permit users to have a simultaneous non-zero balance of the ERC20Upgradeable and RebaseTokenUpgradeable contracts and perform transfers solely with the rebasing-portion of their balance.

Impact:

The USTB token is meant to be a dual-token system supporting non-rebasing and rebasing EIP-20 balances, however, it presently does not properly implement or integrate with its non-rebasing portion causing the overall USTB accounting system to be incorrect.

Example:

/**
 * @notice Updates the state of the contract during token transfers, mints, or burns.
 * @dev This function adjusts the `totalShares` and individual `shares` of `from` and `to` addresses based on the
 * rebasing status (`optOut`). It performs overflow and underflow checks where necessary.
 *
 * @param from The address from which tokens are transferred or burned. Address(0) implies minting.
 * @param to The address to which tokens are transferred or minted. Address(0) implies burning.
 * @param amount The amount of tokens to be transferred.
 */
function _update(address from, address to, uint256 amount) internal virtual override {
    RebaseTokenStorage storage $ = _getRebaseTokenStorage();
    uint256 index = $.rebaseIndex;
    uint256 shares = amount.toShares($.rebaseIndex);
    if (from == address(0)) {
        uint256 totalShares = $.totalShares + shares; // Overflow check required
        _checkRebaseOverflow(totalShares, index);
        $.totalShares = totalShares;
    } else {
        shares = _transferableShares(amount, from);
        unchecked {
            // Underflow not possible: `shares <= $.shares[from] <= totalShares`.
            $.shares[from] -= shares;
        }
    }

    if (to == address(0)) {
        unchecked {
            // Underflow not possible: `shares <= $.totalShares` or `shares <= $.shares[from] <= $.totalShares`.
            $.totalShares -= shares;
        }
    } else {
        unchecked {
            // Overflow not possible: `$.shares[to] + shares` is at most `$.totalShares`, which we know fits into a
            // `uint256`.
            $.shares[to] += shares;
        }
    }

    emit Transfer(from, to, amount);
}

Recommendation:

We advise the code to be revisited and proper support for the non-rebasing portion of the USTB token to be implemented.

Alternatively, we advise the non-rebasing portion of the USTB token to be omitted from the codebase as it is not presently supported.

dkuppitz commented 8 months ago

Resolved in 3b0f43cbe985e394111bdc49941967c683a42fbe.