Yeti Finance
Smart Contract Security Assessment
Dec 21, 2021
![Yeti](/audits/img/logos/yeti.png)
SUMMARY
ABSTRACT
Dedaub was commissioned to perform a security audit on a subset of Yeti Finance’s smart contracts. More specifically, the audit focused on the following files:
- BorrowerOperations.sol
- TroveManager.sol
- TroveManagerLiquidations.sol
- TroveManagerRedemptions.sol
- StabilityPool.sol
The code was audited at commit
62ec9ceea6a1d1cac7843a4361f7bed42d79a35f (main).
The audit also examined any other functionality and contracts that were highly related with the aforementioned contracts. Two auditors worked over the codebase for a little over two weeks.
Yeti is a capital efficient lending protocol, designed to work with multiple collaterals such as Uniswap V2-style LP tokens. It achieves high capital efficiency by borrowing important design elements from Liquity. For instance, liquidations in Yeti happen using a single action that moves collateral from troves (similar to Maker CDPs) to stability providers, and burning the equivalent YUSD position that was liquidated from the stability pool. As opposed to the original Liquity protocol, Yeti allows multiple types of collaterals, with different collateralization ratios for each.
SETTING & CAVEATS
The focus of the audit was strategically limited to the core of the protocol. The time allotted was not sufficient to review the entire protocol, therefore the scope was limited (by the customer) to less than 5000 lines of code, and protocol-level documents. In order to finish the audit in the allotted time, while still maintaining a level of assurance, we:
- reviewed core parts of the code carefully (e.g., Liquidations)
- focused on code differences from the Liquity code
- questioned the economic threats to the high-level protocol
- trusted some of the protocol calculations (e.g., fees).
That is, we put more weight on reviewing threats in the code and the economic design, and less on the correct implementation of calculations of loan and fee parameters. We did this with the understanding that rigorous testing of these calculations is being carried out separately.
PROTOCOL-LEVEL CONSIDERATIONS
The Yeti protocol is more centralized than Liquity, given that the owner (of the Whitelist contract) can change safety ratios (and more). Since safety ratios affect the virtual price of different collateral types, the owner can affect liquidations, redemption order, etc. The intent is that safety ratios will not change during a deployment of the contracts, although the functionality to change them is there.
VULNERABILITIES & FUNCTIONAL ISSUES
This section details issues that affect the functionality of the contract. Dedaub generally categorizes issues according to the following severities, but may also take other considerations into account such as impact or difficulty in exploitation:
- User or system funds can be lost when third-party systems misbehave.
- DoS, under specific conditions.
- Part of the functionality becomes unusable due to a programming error.
- Breaking important system invariants but without apparent consequences.
- Buggy functionality for trusted users where a workaround exists.
- Security issues which may manifest when the system evolves.
Issue resolution includes “dismissed”, by the client, or “resolved”, per the auditors.
CRITICAL SEVERITY
[No critical severity issues]
HIGH SEVERITY
Untrusted callers can change key parameters of the stability pool, since the receiveCollateral function does not check its caller:
function receiveCollateral(address[] memory _tokens, uint256[] memory _amounts)
external
override
{
totalColl.amounts = _leftSumColls(totalColl, _tokens, _amounts);
emit StabilityPoolBalancesUpdated(_tokens, _amounts);
}
MEDIUM SEVERITY
In BorrowerOperations::openTroveRouter, there is a SafeMath expression, whose result is not being assigned:
function openTroveRouter(...) {
...
if (params.j < _routerAVAXIndices.length && params.i ==
_routerAVAXIndices[params.j]) {
// Dedaub: Unassigned SafeMath op
params.avaxSent.sub(_routerAmounts[params.i], "BOps: Router amount is
less than avax sent");
...
}
This means that a transaction may not revert when it should. If the contract were to hold AVAX between transactions (and if the openTroveRouter functionality remains) this can become a serious issue.
In TroveManager::hasPendingRewards, the loop always returns on the first iteration, which does not seem to be the desired behavior.
function hasPendingRewards(address _borrower) public view override returns (bool) {
...
for (uint i = 0; i < assets.length; i++) {
address token = assets[i];
return (rewardSnapshots[_borrower].CollRewards[token] < L_Coll[token]);
}
return false;
}
A fix should look something like the following:
function hasPendingRewards(address _borrower) public view override returns (bool) {
for (uint i = 0; i < assets.length; i++) {
address token = assets[i];
if (rewardSnapshots[_borrower].CollRewards[token] < L_Coll[token])
return true;
}
return false;
}
LOW SEVERITY
There are various places where there’s redundant code:
-
BorrowerOperations::_openTroveInternal:
function _openTroveInternal(...) {
...
vars.YUSDFee; // Dedaub: no-op
...
} -
TroveManagerRedemptions::_redeemCollateralFromTrove:
function _redeemCollateralFromTrove(...) {
...
newColls memory borrowerColls; // Dedaub: unused
...
} -
TroveManagerRedemptions::redeemCollateral:
function redeemCollateral(...) {
...
_requireYUSDBalanceCoversRedemption(
contractsCache.yusdToken,
_redeemer,
_YUSDamount);
...
_requireYUSDBalanceCoversRedemption(
contractsCache.yusdToken,
_redeemer,
_YUSDamount.add(totals.YUSDfee)
);
// Dedaub: second check seems to subsume the first
...
} -
TroveManagerLiquidations::_sendGasCompensation:
function _sendGasCompensation(
IActivePool _activePool,
address _liquidator,
uint256 _YUSD,
address[] memory _tokens,
uint256[] memory _amounts
) internal {
// Dedaub: Unnecessary wrapping
newColls memory _collToSend = newColls(_tokens, _amounts);
if (_YUSD > 0) {
yusdTokenContract.returnFromPool(gasPoolAddress, _liquidator, _YUSD);
}
_activePool.sendCollateralsUnwrap(_liquidator, _collToSend.tokens, _collToSend.amounts, false);
} -
Function BorrowerOperations::_openTroveInternalFinish is not being used anywhere and is internal.
It is recommended that redundant code be removed from the codebase.