Rapid Tin Ram - Potential issues with decimals and last_lsttokenvalueWei in Vault contract may break the core functionalities

[sherlock-admin4]

Rapid Tin Ram

High

Potential issues with decimals and last_lsttokenvalueWei in Vault contract may break the core functionalities

Summary

The variables last_lsttokenvalueWei and decimals in NumaVault.sol are defined as :

// reward extraction variables
    uint256 public last_lsttokenvalueWei;
    // decimals of lst token
    uint256 public immutable decimals;

as it can be seen decimals seem to refering the decimals of lst token. and it is set in the constructor as follow:

constructor(..., uint256 _decimals, ...) {
    decimals = _decimals;
    last_lsttokenvalueWei = oracle.getTokenPrice(decimals);}

The issue here is that decimals is being passed to getTokenPrice(), which appears to be using it to calculate the token price. There are a few issues here:

1. If decimals is 18, it's like requesting the price of 18 token units instead of 1 unit.
2. last_lsttokenvalueWei is initialized based on the oracle price in the constructor but then used in many critical functions without being updated frequently enough. For instance:

function getMaxBorrow() public view returns (uint256) {
    // ...
    uint resultToken = FullMath.mulDiv(
        resultEth,
        decimals,
        last_lsttokenvalueWei  //@audit-issue Using stale price
    );
    // ...
}
......
function liquidateNumaBorrower(...) {
    // ...
    uint minBorrowAmountAllowPartialLiquidationNuma = vaultManager.tokenToNuma(
        minBorrowAmountAllowPartialLiquidation,
        last_lsttokenvalueWei,  //@audit-issue Using stale price
        decimals,
        criticalScaleForNumaPriceAndSellFee
    );
    // ...
}

If we assume decimals name is misleading and it actually refers to quantity of token based on the getTokenPrice function:

 function getTokenPrice(uint256 _amount) external view returns (uint256) {
        return
            tokenToEth(
                _amount,
                feed,
                chainlink_heartbeat,
                IERC20Metadata(token).decimals() //@audit the decimals of the token is fetched here 
            );
    }

there are still some other issues:

1. Stale Price issue:

function buyNoMax(...) internal {
    uint256 numaAmount = vaultManager.tokenToNuma(
        _inputAmount,
        last_lsttokenvalueWei,  // Still using price from constructor
        decimals,               // Amount used for price calculation
        criticalScaleForNumaPriceAndSellFee
    );
}

The price is still only set once in the constructor and used throughout the contract's lifetime.
2) Potential Price Manipulation:

function getMaxBorrow() public view returns (uint256) {
    // ...
    uint resultToken = FullMath.mulDiv(
        resultEth,
        decimals,           // Using amount instead of proper scaling
        last_lsttokenvalueWei  // Using stale price
    );
    // ...
}

Using a stale price makes the system vulnerable to price manipulation.
3) Inconsistent Reference Amount:

// In constructor
last_lsttokenvalueWei = oracle.getTokenPrice(decimals);  // Price for decimals amount

// In liquidation
function liquidateNumaBorrower(...) {
    uint minBorrowAmountAllowPartialLiquidationNuma = vaultManager.tokenToNuma(
        minBorrowAmountAllowPartialLiquidation,  // Different amount
        last_lsttokenvalueWei,                   // Price based on different amount
        decimals,
        criticalScaleForNumaPriceAndSellFee
    );
}

Root Cause

The way of setting the variables last_lsttokenvalueWei and decimals in NumaVault.sol :

// In NumaVault constructor
constructor(..., uint256 _decimals, ...) {
    decimals = _decimals;  
    last_lsttokenvalueWei = oracle.getTokenPrice(decimals);  
}

Internal pre-conditions

The admin deploys the Vault contract

External pre-conditions

Any external function call that uses the mentioned variables

Attack Path

Whenever a call happens to buy /sell or any other functions that are related to decimals and last_lsttokenvalueWei , the user or platform will be affected.

Impact

There are a bunch of different impacts:
Potential price Manipulation
Price Staleness
The stale prices may lead to Over/Under-collateralization
Incorrect liquidation triggers
Loss of funds

PoC

No response

Mitigation

It is not clear what is the intended usage of decimals. Based on the usage of decimals make the necessary adjustment for both variables so that the mentioned issues above do not exist.