Omniscia Teahouse Finance Audit

TeaVaultV3PairOracle Manual Review Findings

TeaVaultV3PairOracle Manual Review Findings

TVP-01M: External Security Requirements

Description:

The TeaVaultV3PairOracle::_getTwap function makes several security assumptions about the _teaVaultV3Pair it interacts with which, if broken, will cause the TeaVaultV3PairOracle::_getTwap function to yield incorrect prices.

Impact:

As the finding pertains to an out-of-scope contract, pinpointing its exact impact is impossible.

Example:

contracts/oracle/TeaVaultV3PairOracle.sol
135function _getTwap(
136    ITeaVaultV3Pair _teaVaultV3Pair,
137    TeaVaultV3PairInfo memory _teaVaultV3PairInfo
138) internal view returns (
139    uint256 price
140) {
141    if (address(_teaVaultV3PairInfo.token0) == address(0)) revert AssetNotEnabled();
142    IAssetOracle _baseAssetOracle = baseAssetOracle;
143    uint256 price0 = _teaVaultV3PairInfo.token0 != baseAsset
144        ? _baseAssetOracle.getTwap(_teaVaultV3PairInfo.token0)
145        : 10 ** baseAssetOracleDecimals;
146    uint256 price1 = _teaVaultV3PairInfo.token1 != baseAsset
147        ? _baseAssetOracle.getTwap(_teaVaultV3PairInfo.token1)
148        : 10 ** baseAssetOracleDecimals;
149

150    uint160 sqrtPriceX96 = uint160(
151        (_sqrt(uint256(price0) * 10 ** _teaVaultV3PairInfo.token1Decimals) << 96) /
152        _sqrt(uint256(price1) * 10 ** _teaVaultV3PairInfo.token0Decimals)
153    );
154

155    uint256 token0Balance = _teaVaultV3Pair.getToken0Balance();
156    uint256 token1Balance = _teaVaultV3Pair.getToken1Balance();
157

158    ITeaVaultV3Pair.Position[] memory allPositions = _teaVaultV3Pair.getAllPositions();
159    for (uint256 i; i < allPositions.length; i = i + 1) {
160        // calculate token0 and token1 amount of each position based on converted oracle price
161        (uint256 amount0, uint256 amount1) = LiquidityAmounts.getAmountsForLiquidity(
162            sqrtPriceX96,
163            TickMath.getSqrtRatioAtTick(allPositions[i].tickLower),
164            TickMath.getSqrtRatioAtTick(allPositions[i].tickUpper),
165            allPositions[i].liquidity
166        );
167

168        token0Balance = token0Balance + amount0;
169        token1Balance = token1Balance + amount1;
170    }
171    (, , uint256 fee0, uint256 fee1) = _teaVaultV3Pair.allPositionInfo();
172    token0Balance = token0Balance + fee0;
173    token1Balance = token1Balance + fee1;
174    
175    // totalValue = sigma(twap * (amount / 10 ^ baseAssetDecimals)) / 10 ^ twapDecimals
176    price = token0Balance.mulDiv(price0, 10 ** _teaVaultV3PairInfo.token0Decimals) + 
177        token1Balance.mulDiv(price1, 10 ** _teaVaultV3PairInfo.token1Decimals);
178    // shareTokenTwap = (totalValue / (shareTotalSupply / 10 ^ shareDecimals)) * 10 ^ DECIMALS
179    price = price.mulDiv(
180        10 ** (DECIMALS + ERC20(address(_teaVaultV3Pair)).decimals()),
181        ERC20(address(_teaVaultV3Pair)).totalSupply() * 10 ** baseAssetOracleDecimals
182    );
183}

Recommendation:

Given that the _teaVaultV3Pair implementation is not in scope of the audit, we strongly advise the Teahouse Finance team to confirm that the asset amounts yielded by the ITeaVaultV3Pair::getToken0Balance, ITeaVaultV3Pair::getToken1Balance, ITeaVaultV3Pair::getAllPositions, and ITeaVaultV3Pair::allPositionInfo do not contain any overlap as such a case would cause the TWAP to misbehave.

Alleviation (302b96f324a88038a0872015466cd43783c14543):

The Teahouse Finance team stated that the TeaVaultV3Pair should be considered secure for all intents and purposes of the audit and as such, we consider this exhibit acknowledged and have treated the TeaVaultV3Pair contract as a black box during our audit.

TVP-02M: Inexistent Validation of Array Lengths

Description:

The TeaVaultV3PairOracle::getBatchValueWithTwap function, in contrast to its TeaVaultV3PairOracle::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:

contracts/oracle/TeaVaultV3PairOracle.sol
73/// @inheritdoc IAssetOracle
74function getBatchValue(
75    address[] calldata _assets,
76    uint256[] calldata _amounts
77) external override view returns (
78    uint256[] memory values
79) {
80    if (_assets.length != _amounts.length) revert BatchLengthMismatched();
81

82    values = new uint256[](_assets.length);
83    for (uint256 i; i < _assets.length; i = i + 1) {
84        values[i] = _getValue(_assets[i], _amounts[i]);
85    }
86}
87

88/// @inheritdoc IAssetOracle
89function getValueWithTwap(address _asset, uint256 _amount, uint256 _twap) external override view returns (uint256 value) {
90    return _getValueWithTwap(_asset, _amount, _twap);
91}
92

93/// @inheritdoc IAssetOracle
94function getBatchValueWithTwap(
95    address[] calldata _assets,
96    uint256[] calldata _amounts,
97    uint256[] calldata _twaps
98) external override view returns (
99    uint256[] memory values
100) {
101    values = new uint256[](_assets.length);
102

103    for (uint256 i; i < _assets.length; i = i + 1) {
104        values[i] = _getValueWithTwap(_assets[i], _amounts[i], _twaps[i]);
105    }
106}

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.

TVP-03M: Unsafe Casting Operation

Description:

The result of a complex calculation within TeaVaultV3PairOracle::_getTwap is being cast to a uint160 variable without any bound checks, permitting an undetected casting overflow to occur.

Impact:

The likelihood of an overflow occurring depends on the volatility of the assets added to the base asset oracle and thus is of low likelihood.

Example:

contracts/oracle/TeaVaultV3PairOracle.sol
150uint160 sqrtPriceX96 = uint160(
151    (_sqrt(uint256(price0) * 10 ** _teaVaultV3PairInfo.token1Decimals) << 96) /
152    _sqrt(uint256(price1) * 10 ** _teaVaultV3PairInfo.token0Decimals)
153);

Recommendation:

We advise the cast to be safely performed, ensuring the calculated value does not exceed the maximum supported by the uint160 data type (i.e. type(uint160).max).

To note, usage of 0.8.X pragma versions do not protect against casting overflows as these remain unchecked.

Alleviation (302b96f324a88038a0872015466cd43783c14543):

A safe casting operation is now performed instead, ensuring that the resulting sqrtPriceX96 has been safely cast to its uint160 data type.

Prev

Swapper.sol (SRE-M)

Next

TeaVaultV3Portfolio.sol (TVV-M)

On this page