branch_indigo - Wrong Price will be Returned When Asset is PToken for WstETH

[sherlock-admin]

branch_indigo

high

Wrong Price will be Returned When Asset is PToken for WstETH

Summary

Iron Bank allows a PToken market to be created for an underlying asset in addition to a lending market. PTokens can be counted as user collaterals and their price is fetched based on their underlying tokens. However, wrong price will return when PToken's underlying asset is WstETH.

Vulnerability Detail

Retrieving price for WstETH is a 2 step process. WstETH needs to be converted to stETH first, then converted to ETH/USD. This is properly implemented when the market is WstETH through checking if (asset==wsteth). But when PToken market is created for WstETH, this check will by bypassed because PToken contract address will be different from wsteth address.

PToken market price is set through _setAggregators() in PriceOracle.sol where base and quote token address are set and tested before writing into aggregators array. And note that quote token address can either be ETH or USD. When asset price is accessed through getPrice(), if the input asset is not wsteth address, aggregators is directly pulled to get chainlink price denominated in ETH or USD.

//PriceOracle.sol
//_setAggregators()
                require(
                    aggrs[i].quote == Denominations.ETH ||
                        aggrs[i].quote == Denominations.USD,
                    "unsupported quote"
                );

//PriceOracle.sol
    function getPrice(address asset) external view returns (uint256) {
        if (asset == wsteth) {
            uint256 stEthPrice = getPriceFromChainlink(
                steth,
                Denominations.USD
            );
            uint256 stEthPerToken = WstEthInterface(wsteth).stEthPerToken();
            uint256 wstEthPrice = (stEthPrice * stEthPerToken) / 1e18;
            return getNormalizedPrice(wstEthPrice, asset);
        }
        AggregatorInfo memory aggregatorInfo = aggregators[asset];
        uint256 price = getPriceFromChainlink(
            aggregatorInfo.base,
            aggregatorInfo.quote
        );
       ...

This creates a problem for PToken for WstETH, because if (asset==wsteth) will be bypassed and chainlink aggregator price will be returned. And chainlink doesn't have a direct price quote of WstETH/ETH or WstETH/USD, only WstETH/stETH or stETH/USD. This means most likely aggregator price for stETH/USD will be returned as price for WstETH.

Since stETH is a rebasing token, and WstETH:stETH is not 1 to 1, this will create a wrong valuation for users holding PToken for WstETH as collaterals.

Impact

Since users holding PToken for WstETH will have wrong valuation, this potentially creates opportunities for malicious over-borrowing or unfair liquidations, putting the protocol at risk.

Code Snippet

https://github.com/sherlock-audit/2023-05-ironbank/blob/main/ib-v2/src/protocol/oracle/PriceOracle.sol#L43

Tool used

Manual Review

Recommendation

In getPrice(), consider adding another check whether the asset is PToken and its underlying asset is WstETH. If true, use the same bypass for pricing.

[bzpassersby]

Escalate for 10 USDC I think this is wrongly excluded. The report describes a clear vulnerability that the current Oracle implementation doesn't take into account PToken for WstETH. stETH is a rebasing token ,which is correctly factored in oracle implementation when the market asset is for WstETH. However, when a PToken is created for WstETH, if(asset==wsteth) will be bypassed. Since chainlink doesn't have a direct feed for WstETH/ETH or WstETH/USD. Wrong price will be returned.

[sherlock-admin]

Escalate for 10 USDC I think this is wrongly excluded. The report describes a clear vulnerability that the current Oracle implementation doesn't take into account PToken for WstETH. stETH is a rebasing token ,which is correctly factored in oracle implementation when the market asset is for WstETH. However, when a PToken is created for WstETH, if(asset==wsteth) will be bypassed. Since chainlink doesn't have a direct feed for WstETH/ETH or WstETH/USD. Wrong price will be returned.

You've created a valid escalation for 10 USDC!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

[0xffff11]

Issue seems valid to me. I would judge it as a med because seems that it is for a only a specific pair, quite an edgecases. Thoughts @ibsunhub ?

[ibsunhub]

We're ok with med.

Although we don't decide that if we will support wstETH's PToken, it's our oversight to handle of the price of wstETH's PToken in the price oracle.

[ibsunhub]

fix: https://github.com/ibdotxyz/ib-v2/pull/57

[hrishibhat]

Result: Medium Unique Considering this issue a valid medium based on the above comments.

[sherlock-admin]

Escalations have been resolved successfully!

Escalation status:

• bzpassersby: accepted

[IAm0x52]

Fix looks good. PTokens now always use their underlying token when determining their price