Radiant - GLP

The totalAssets() function in GLPStrategy.sol assumes all basket tokens have 18 decimals when calculating their USD value, leading to incorrect asset valuations for tokens with different decimal places.

• tokenBalance is the raw balance of the token (in its native decimals)
• price is the USD price from the oracle router (with decimals decimal places)
• decimals refers to the price decimals, not the token decimals

The calculation doesn't normalize the token balance to a standard decimal base before multiplying by price. For tokens with fewer than 18 decimals (like USDC with 6 decimals), this results in massive overvaluation. For tokens with more than 18 decimals, this results in undervaluation.

High - This bug causes severe miscalculation of total assets under management.

Normalize token balances to 18 decimals before calculating their USD value:

function totalAssets() public view override returns (uint256) {
    uint256 total = weth.balanceOf(GUARDIAN_MULTISIG);
    if (basketTokens.length == 0) return total;

    for (uint256 i = 0; i < basketTokens.length; i++) {
        address token = basketTokens[i];
        uint256 tokenBalance = IERC20(token).balanceOf(GUARDIAN_MULTISIG);

        // Normalize token balance to 18 decimals, then apply price
        if (tokenBalance == 0) continue;
        uint256 normalizedBalance = tokenBalance * (10 ** (18 - IERC20Metadata(token).decimals()));

        (uint256 price, uint8 priceDecimals) = oracleRouter.getAssetPrice(token);
        if (price == 0) continue;

        total += normalizedBalance.mulDiv(price, 10 ** priceDecimals);
    }
    return total;
}

Fixed in: PR#7

The removeStrategy() function removes the strategy reference without retrieving the funds managed by the strategy, potentially making vault assets inaccessible.

The current removeStrategy() implementation (lines 61-66) only:

1. Revokes WETH approval from the strategy
2. Sets strategy to address(0)
3. Emits an event

However, it fails to call strategy.divest() to pull back funds before nullifying the strategy reference. The vault's totalAssets() function (line 100) includes strategy assets:

function totalAssets() public view override returns (uint256) {
    return IERC20(asset()).balanceOf(address(this)) + strategy.totalAssets();
}

After removeStrategy(), this will revert when calling strategy.totalAssets() on a zero address.

Medium.

• Stranded funds: Assets managed by the removed strategy become inaccessible to the vault until a new strategy is added back and pointing to the same vault.
• Broken accounting: totalAssets() reverts after strategy removal, breaking deposit/withdrawal calculations
• User impact: Withdrawals may fail due to insufficient liquid assets in the vault, even though funds exist in the removed strategy

• Modify removeStrategy() to pull all strategy funds back to the vault before removal. It's important to call divest() with the entire deposited amount to keep the totalAssets() in balance. The code should check that the strategy totalAssets is zero.
• Fix totalAssets() to handle the case when no strategy is configured to prevent reverts.

Added strategy update checks totalAssets consistency in PR#7

The price of a share can increase instantaneously via different ways:

• WETH transfer to the vault, the strategy, or the multisig; this is an expected and known behaviour.
• If a token is added to the basket and the multisig already owns this asset.
• When the protocol burns dao shares.

Low. This issue only occurs if a mistake is made in admin operations.

Add checks to minimize the risk of operational mistakes.

• Make sure there are no tokens in the multisig when adding it to the pool:

 function addBasketToken(address token) external onlyOwner {
       // Check if multisig holds this token
       uint256 existingBalance = IERC20(token).balanceOf(GUARDIAN_MULTISIG);
       if (existingBalance > 0 && token != RDNT) {
           // Either revert or handle the existing balance appropriately
           revert("Cannot add tokens with existing multisig balance");
       }
       // ... rest of function
   }

• Burn shares should only allow to burn an amount of shares with limited impact. As an example, the amount of shares burned should not increase the share price by more than 1%. This can be done with the following code:

    function burnDaoShares(uint256 shares) external onlyOwner whenBootstrapped {
        if (shares == 0) revert Errors.InsufficientShares();
        if (shares > balanceOf(DAO_TREASURY)) revert Errors.InsufficientShares();
        
        // Limit burn to maximum 1% price impact
        // Math: to keep price increase <= 1%, max burnable = totalSupply() / 101
        uint256 maxBurnableShares = totalSupply() / 101;
        if (shares > maxBurnableShares) revert Errors.ExcessivePriceImpact();
        
        _burn(DAO_TREASURY, shares); // internal ERC20 burn, no assets move
        emit DAOSharesBurned(DAO_TREASURY, shares);
    }

• Fixed in PR#8
• Acknowledge to burn shares. This will be done carefully

The GLPStrategy.totalAssets function may double-count the WETH balance if WETH is mistakenly added to the basketTokens array

function addBasketToken(address token) external onlyOwner {
    if (token == address(0)) revert Errors.NullAddress();
    for (uint256 i = 0; i < basketTokens.length; i++) {
        if (basketTokens[i] == token) {
            revert Errors.TokenAlreadyExists();
        }
    }
    (uint256 price,) = oracleRouter.getAssetPrice(token);
    if (price == 0) revert Errors.InvalidPrice();
    basketTokens.push(token);
    emit BasketTokenAdded(token);
}

The GLPStrategy.totalAssets() function double-counts WETH balances when WETH is included in basketTokens. It first adds the total WETH balance, then adds it again during the basketTokens loop, inflating the reported NAV.
This inflated NAV causes incorrect share pricing in GLPVault functions. New depositors receive fewer shares than they should, while existing shareholders can redeem at inflated prices to extract excess value from the vault.

Low - Only an issue if WETH gets added to the basket of tokens

Block weth from be added in addBasketToken(address token)

Fixed in: 1a7682.

The removeBasketToken function allows the owner to remove tokens from the basket without checking if the protocol still holds balances. This can cause unexpected behaviour by deflating the vault's NAV.

function removeBasketToken(uint256 index) external onlyOwner {
    if (index >= basketTokens.length) revert Errors.IndexOutOfBounds();
    address token = basketTokens[index];
    basketTokens[index] = basketTokens[basketTokens.length - 1];
    basketTokens.pop();
    emit BasketTokenRemoved(token);
}

The vault's NAV is calculated by totalAssets, which sums the value of balances in basketTokens. The removeBasketToken function removes tokens by index without checking if their balance is zero. Removing a token with significant balance excludes its value from future NAV calculations, artificially deflating the gLP share price.

Low - Causes unexpected deflation of gLP share price.

Add a balance check to removeBasketToken to prevent delisting assets that still have value backing the vault shares. Or think of a way to handle this more gracefully.

Fixed in PR#6

The burnDaoShares function in GLPVault lacks checks to prevent the total supply of shares from being reduced to a critically low level (or zero). This allows an owner to create conditions for an ERC4626 inflation attack,

    function burnDaoShares(uint256 shares) external onlyOwner whenBootstrapped {
        if (shares == 0) revert Errors.InsufficientShares();
        if (shares > balanceOf(DAO_TREASURY)) revert Errors.InsufficientShares();
        _burn(DAO_TREASURY, shares); // internal ERC20 burn, no assets move
        emit DAOSharesBurned(DAO_TREASURY, shares);
    }

The owner can reduce totalSupply() by DAO shares, creating a scenario where shares are low but totalAssets() remains high, following a depositor mints a small amount of shares at an inflated value.

Low - Under the owner's control and unlikely to happen, but can be easily remedied to prevent it.

Modify the Bootstrap function to mint a small number of tokens to a seperate/dead address

We acknowledge this risk and won't remediate this.

The latestAnswer() function in InverseWETHChainlinkAdapter.sol returns int256 but prices are never negative. The function validates that both price feeds are positive (line 53) and performs calculations using uint256.

Informational.

Consider using uint256 for price return.

Acknowledged.

The variable is being cast to its own type.

File: src/GLPStrategy.sol

64: emit OracleRouterSet(address(_router));

src/GLPStrategy.sol#L64

Informational

Remove the unnecessary cast

Fixed in: d72c9a4.

The bootstrap() function mints shares equal to totalAssets(), but if called when the strategy hasn't been configured to include RdnT as a basketTokens, it will mint zero shares, but it will be bootstrapped.

Informational.

Make sure more than zero shares are minted to the treasury.

Fixed in: 91ce51.

The whenBootstrapped modifier on withdraw() and redeem() functions (lines 132, 143) is redundant. Users can only acquire shares when the vault is already bootstrapped via deposit() or mint(), so any user calling withdrawal functions implies that the vault is already bootstrapped.

Gas savings.

Remove whenBootstrapped modifier from both functions

Ack, just for consistency I would leave it there, since it's arbitrum and gas costs nothing

The divest() function performs redundant balance checking. safeTransferFrom() either transfers the exact amount or reverts - no partial transfers occur.

Gas savings.

Simplify the function by removing redundant balance checking:

function divest(uint256 amount) external override nonReentrant onlyVault returns (uint256) {
    weth.safeTransferFrom(GUARDIAN_MULTISIG, address(this), amount);
    weth.safeTransfer(vault, amount);
    return amount;
}

This achieves the same functionality with lower gas costs and clearer logic.

Fixed at 490ad8.

File: src/oracles/adapters/ValidatedChainlinkAdapterWithSequencer.sol

13: uint256 public constant UPDATE_PERIOD = 86400;

ValidatedChainlinkAdapterWithSequencer.sol#L13

Gas savings.

Remove the not used constant.

Fixed in: c30395d