AssetOracle Manual Review Findings

AOE-01M: Potential Misconception of Asset Invariance

Type	Severity	Location
Standard Conformity		AssetOracle.sol:L157-L160

Description:

The AssetOracle will treat the baseAsset as being "one-to-one" with the expected price the oracle checks against. While this may be desirable in case all prices are meant to be measured in relation to the baseAsset (i.e. USDC), it should be noted that assets like USDC, USDT, DAI, etc. are never at parity with the US Dollar and tend to fluctuate.

Impact:

The severity of this exhibit cannot be assessed as the current system may be behaving as designed.

Example:

Copy
154function _getTwap(PoolInfo[] memory _poolInfoChain) internal view returns (uint256 price) {
155    if ((_poolInfoChain.length) == 0) revert AssetNotEnabled();
156
157    price = 10 ** DECIMALS;
158    if (address(_poolInfoChain[0].pool) == address(1)) {
159        return price;
160    }

Recommendation:

We advise the Teahouse Finance team to evaluate this and make changes as necessary, given that the system should not rely on the assumption that a stablecoin is one-to-one with the US Dollar.

Alleviation (302b96f324a88038a0872015466cd43783c14543):

The Teahouse Finance team evaluated this exhibit and stated that this behaviour is by design as the function is solely utilized in calculating the performance fee. As such, we consider the exhibit acknowledged.

AOE-02M: Inexistent Protection of Multiplication Overflow

Type	Severity	Location
Mathematical Operations		AssetOracle.sol:L177

Description:

The AssetOracle::_getTwap function can result in an overflow failure under normal operating conditions of a Uniswap V3 pool due to its code not accounting for an abnormally high sqrtPriceX96 value.

Impact:

The AssetOracle::_getTwap will fail to calculate a TWAP for a valid Uniswap V3 pool under certain scenarios whereby the sqrtPriceX96 is significantly high, resulting in an irreversible Denial-of-Service.

Example:

Copy
175if (_poolInfo.assetIsToken0) {
176    // (sqrtX96 / 2 ^ 96) ^ 2 * 10 ^ (decimals0 - decimals1) * 10 ^ DECIMALS
177    relativePrice = sqrtPriceX96.mulDiv(sqrtPriceX96, 1 << 96);
178    relativePrice = _poolInfo.decimals0 > _poolInfo.decimals1 ? 
179        relativePrice.mulDiv(10 ** (DECIMALS + _poolInfo.decimals0 - _poolInfo.decimals1), 1 << 96) :
180        relativePrice.mulDiv(10 ** DECIMALS, (1 << 96) * 10 ** (_poolInfo.decimals1 - _poolInfo.decimals0));
181}
182else {

Recommendation:

We advise the code to mimic the standard Uniswap V3 library and specifically the OracleLibrary::getQuoteAtTick function, performing a different calculation if sqrtPriceX96 ** 2 would overflow.

Alleviation (302b96f324):

The Teahouse Finance team evaluated this exhibit and stated that the present accuracy of the code is sufficient for most intents and purposes.

We would like to rehash that the same segment of code as present in the Uniswap V3 codebase differs and is equipped to handle a would-be-overflowing sqrtPriceX96 value.

Given that the Uniswap V3 codebase itself takes care of this, we strongly advise the Teahouse Finance team to revisit this particular exhibit.

Alleviation (a337648498):

Our recommendation was properly applied, adopting the logic of the OracleLibrary of Uniswap V3 for the purposes of the AssetOracle and properly handling a sqrtPriceX96 that will not overflow when multiplied by itself.

AOE-03M: Inexistent Validation of Array Lengths

Type	Severity	Location
Input Sanitization		AssetOracle.sol:L96, L111-L113

Description:

The AssetOracle::getBatchValueWithTwap function, in contrast to its AssetOracle::getBatchValue counterpart, does not validate the lengths of its input arrays.

Impact:

Incorrectly formatted arrays can lead to the exploitation of compiler-level vulnerabilities as well as undefined behaviour due to an out-of-bound read operation.

Example:

Copy
89/// @inheritdoc IAssetOracle
90function getBatchValue(
91    address[] calldata _assets,
92    uint256[] calldata _amounts
93) external override view returns (
94    uint256[] memory values
95) {
96    if (_assets.length != _amounts.length) revert BatchLengthMismatched();
97
98    values = new uint256[](_assets.length);
99    for (uint256 i; i < _assets.length; i = i + 1) {
100        values[i] = _getValue(_assets[i], _amounts[i]);
101    }
102}
103
104/// @inheritdoc IAssetOracle
105function getValueWithTwap(address _asset, uint256 _amount, uint256 _twap) external override view returns (uint256 value) {
106    return _getValueWithTwap(_asset, _amount, _twap);
107}
108
109/// @inheritdoc IAssetOracle
110function getBatchValueWithTwap(
111    address[] calldata _assets,
112    uint256[] calldata _amounts,
113    uint256[] calldata _twaps
114) external override view returns (
115    uint256[] memory values
116) {
117    values = new uint256[](_assets.length);
118
119    for (uint256 i; i < _assets.length; i = i + 1) {
120        values[i] = _getValueWithTwap(_assets[i], _amounts[i], _twaps[i]);
121    }
122}

Recommendation:

We advise the input arrays to be validated, ensuring that proper length arrays have been passed into the function.

Alleviation (302b96f324a88038a0872015466cd43783c14543):

The input array lengths are now properly validated, yielding a BatchLengthMismatched error in case they do not equal each other.