sherlock-audit / 2023-03-Y2K-judging

7 stars 1 forks source link

0xnirlin - Oracle is tracked per token using mapping that can lead to unexpected results when more markets are created. #471

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

0xnirlin

medium

Oracle is tracked per token using mapping that can lead to unexpected results when more markets are created.

Summary

Vulnerability Detail

https://github.com/sherlock-audit/2023-03-Y2K/blob/main/Earthquake/src/v2/VaultFactoryV2.sol#L26 https://github.com/sherlock-audit/2023-03-Y2K/blob/main/Earthquake/src/v2/VaultFactoryV2.sol#L72

The protocol assumes that there is a canonical asset to compare pegged assets to, so oracles are tracked only by the pegged asset. However, for some assets (like BTC), there is no clear canonical asset, and the result is that tracking tokenToOracle is not sufficient.

When there is a conflict in tokenToOracle, the protocol responds by skipping the assignment and keeping the old value:

if (tokenToOracle[_token] == address(0)) {
    tokenToOracle[_token] = _oracle;
}

The result of this is that the protocol may define a new pair with a new oracle, and have it silently skip it and use a non-matching oracle. As an example:

The admins start with an implementation of ibBTC-wBTC, deploying the oracle
This is set as tokenToOracle[ibBTC]
Later, the admins create a new pair for ibBTC-renBTC, deploying a new oracle
The protocol silently skips this assignment and uses the ibBTC-wBTC oracle

This can produce incorrect results, for example if wBTC appreciates relative to the other two, or if both ibBTC and renBTC depeg.

Impact

Oracles are tracked by individual token, instead of by pair. Because for some tokens (ie BTC) it is unclear which implementation is the canonical one to compare others to, this can result in a situation where different pairs (ie ibBTC-wBTC and ibBC-renBTC) using the same oracle, which will be incorrect for one of them.

Code Snippet

https://github.com/sherlock-audit/2023-03-Y2K/blob/main/Earthquake/src/v2/VaultFactoryV2.sol#L26 https://github.com/sherlock-audit/2023-03-Y2K/blob/main/Earthquake/src/v2/VaultFactoryV2.sol#L72

Tool used

Manual Review

Recommendation

Change tokenToOracle to represent the pair of tokens, either by creating a Pair struct as the key, or by nesting a mapping inside of another mapping.

Duplicate of #321