Omniscia Tangible Audit

RebaseTokenUpgradeable Manual Review Findings

RebaseTokenUpgradeable Manual Review Findings

RTU-01M: EIP-20 Discrepancy of Rebase-Based System

Description:

The RebaseTokenUpgradeable::_update function will incorrectly update its stated based on the input amount to be transferred as it will not ensure that the conversions it performs do not truncate.

In detail, it is possible for the amount emitted as being transferred in the Transfer event to be greater-than the actual amount transferred based on the RebaseTokenMath::toShares conversions of the RebaseTokenUpgradeable::_update and RebaseTokenUpgradeable::_transferableShares functions.

Impact:

A non-conformant EIP-20 token is considered a significant vulnerability as the token is meant to seamlessly interoperate with off-chain EIP-20 systems.

Example:

src/RebaseTokenUpgradeable.sol
200function _update(address from, address to, uint256 amount) internal virtual override {
201    RebaseTokenStorage storage $ = _getRebaseTokenStorage();
202    uint256 index = $.rebaseIndex;
203    uint256 shares = amount.toShares($.rebaseIndex);
204    if (from == address(0)) {
205        uint256 totalShares = $.totalShares + shares; // Overflow check required
206        _checkRebaseOverflow(totalShares, index);
207        $.totalShares = totalShares;
208    } else {
209        shares = _transferableShares(amount, from);
210        unchecked {
211            // Underflow not possible: `shares <= $.shares[from] <= totalShares`.
212            $.shares[from] -= shares;
213        }
214    }
215

216    if (to == address(0)) {
217        unchecked {
218            // Underflow not possible: `shares <= $.totalShares` or `shares <= $.shares[from] <= $.totalShares`.
219            $.totalShares -= shares;
220        }
221    } else {
222        unchecked {
223            // Overflow not possible: `$.shares[to] + shares` is at most `$.totalShares`, which we know fits into a
224            // `uint256`.
225            $.shares[to] += shares;
226        }
227    }
228

229    emit Transfer(from, to, amount);
230}

Recommendation:

We advise the code to instead convert the actual transferred shares back to the amount, emitting the Transfer event with the actual transferred amount rather than the initially desired one.

Alleviation (47fbf62bbbf2409ff0baf9a18a2945466cb2a576):

The event's amount argument was corrected to utilize the shares value converted to tokens based on the current index, alleviating this exhibit.

RTU-02M: Inexistent Support of Rebasing Status

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:

src/RebaseTokenUpgradeable.sol
191/**
192 * @notice Updates the state of the contract during token transfers, mints, or burns.
193 * @dev This function adjusts the `totalShares` and individual `shares` of `from` and `to` addresses based on the
194 * rebasing status (`optOut`). It performs overflow and underflow checks where necessary.
195 *
196 * @param from The address from which tokens are transferred or burned. Address(0) implies minting.
197 * @param to The address to which tokens are transferred or minted. Address(0) implies burning.
198 * @param amount The amount of tokens to be transferred.
199 */
200function _update(address from, address to, uint256 amount) internal virtual override {
201    RebaseTokenStorage storage $ = _getRebaseTokenStorage();
202    uint256 index = $.rebaseIndex;
203    uint256 shares = amount.toShares($.rebaseIndex);
204    if (from == address(0)) {
205        uint256 totalShares = $.totalShares + shares; // Overflow check required
206        _checkRebaseOverflow(totalShares, index);
207        $.totalShares = totalShares;
208    } else {
209        shares = _transferableShares(amount, from);
210        unchecked {
211            // Underflow not possible: `shares <= $.shares[from] <= totalShares`.
212            $.shares[from] -= shares;
213        }
214    }
215

216    if (to == address(0)) {
217        unchecked {
218            // Underflow not possible: `shares <= $.totalShares` or `shares <= $.shares[from] <= $.totalShares`.
219            $.totalShares -= shares;
220        }
221    } else {
222        unchecked {
223            // Overflow not possible: `$.shares[to] + shares` is at most `$.totalShares`, which we know fits into a
224            // `uint256`.
225            $.shares[to] += shares;
226        }
227    }
228

229    emit Transfer(from, to, amount);
230}

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.

Alleviation (47fbf62bbbf2409ff0baf9a18a2945466cb2a576):

Proper non-rebasing balance support has been introduced to the system in the form of the optOut system and significant updates in the RebaseTokenUpgradeable::_update function specifically, alleviating this exhibit.

Prev

LayerZeroRebaseTokenUpgradeable.sol (LZR-M)

Next

CrossChainRebaseTokenUpgradeable.sol (CCR-C)

On this page