You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
This repository has been archived by the owner on Nov 26, 2023. It is now read-only.
sherlock-admin opened this issue
May 24, 2023
· 0 comments
Labels
DuplicateA valid issue that is a duplicate of an issue with `Has Duplicates` labelHighA valid High severity issueRewardA payout will be made for this issue
Direct transfer of USSD or DAI to the uniPool causes rebalance to malfunction
Summary
Because of reliance on DAI.balanceOf(uniPool) and USSD.balanceOf(uniPool) in USSDRebalancer.rebalance, direct transfer of DAI to the pool will cause rebalance to buy or mint wrong amounts of USSD.
Vulnerability Detail
First, let's notice that, when calling USSDRebalancer.rebalance(), we use sqrtPriceX96 from uniPool.slot0() to decide whether to call BuyUSSDSellCollateral, SellUSSDBuyCollateral or do nothing. To see this, examine the following lines of code: 1, 2, 3, 4
However, to choose how much USSD to buy for DAI or, in the 2nd case, how much USSD to mint and then sell for DAI, we use USSD.balanceOf(uniPool) and DAI.balanceOf(uniPool). To see this, examine the following lines of code: 1, 2, 3, 4.
Sending DAI (or USSD) to the pool directly influences DAI.balanceOf(uniPool) but doesn't influence sqrtPriceX96 of the pool.
Moreover, notice that Uniswap V3 pools (unlike Uniswap V2 pairs) don't have the sync mechanism that would allow to easily set reserves in the pool to actual balances of tokens in the pool.
This can lead to DAIamount / 1e12 - USSDamount values passed to mintRebalancer or BuyUSSDSellCollateral to be different than expected. Relevant lines of code: 1, 2.
Let's consider a situation where someone has sent some DAI to the pool directly. In this case, it's possible that the price of 1 USSD in the pool is less than 1 DAI (and the threshold is exceeded) but USSDamount - DAIamount / 1e12 is negative (or smaller than expected) because the DAI previously sent directly to the pool didn't influence the price.
If that's the case, then call to rebalance() will revert becasue of underflow in BuyUSSDSellCollateral((USSDamount - DAIamount / 1e12) / 2);. This will DoS the rebalance() functionality of USSD, and likely will cause USSD to depeg without option to use collateral in the USSD contract to recover the peg. This will, in turn, cause USSD to lose its value because not many people are comfortable with holding depegged stablecoins.
Even if USSDamount - DAIamount / 1e12 is not negative, just smaller than expected, then too little USSD will be bought to fully recover the peg.
Now let's consider a situation, in which the price of 1 USSD more than 1 DAI but someone has sent DAI directly to the pool before. This will cause minting more USSD than expected in this line and overshoot the 1 DAI price for 1 USSD. This situation is intended to be prevented as we can read in the whitepaper or in comments in this line:
// never sell too much USSD for DAI so it 'overshoots' (becomes more in quantity than DAI on the pool)
// otherwise could be arbitraged through mint/redeem`
Impact
If someone sends DAI to the pool directly (by transfer, not by swap), then USSD will depeg because of USSDRebalancer.rebalance() rebalancing too little or even reverting.
Another possibility is that selling USSD for DAI in rebalance will overshoot and create unwelcome arbitrage opportunities.
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Labels
DuplicateA valid issue that is a duplicate of an issue with `Has Duplicates` labelHighA valid High severity issueRewardA payout will be made for this issue
smiling_heretic
medium
Direct transfer of USSD or DAI to the
uniPool
causesrebalance
to malfunctionSummary
Because of reliance on
DAI.balanceOf(uniPool)
andUSSD.balanceOf(uniPool)
inUSSDRebalancer.rebalance
, direct transfer of DAI to the pool will causerebalance
to buy or mint wrong amounts of USSD.Vulnerability Detail
First, let's notice that, when calling
USSDRebalancer.rebalance()
, we usesqrtPriceX96
fromuniPool.slot0()
to decide whether to callBuyUSSDSellCollateral
,SellUSSDBuyCollateral
or do nothing. To see this, examine the following lines of code: 1, 2, 3, 4However, to choose how much USSD to buy for DAI or, in the 2nd case, how much USSD to mint and then sell for DAI, we use
USSD.balanceOf(uniPool)
andDAI.balanceOf(uniPool)
. To see this, examine the following lines of code: 1, 2, 3, 4.Sending DAI (or USSD) to the pool directly influences
DAI.balanceOf(uniPool)
but doesn't influencesqrtPriceX96
of the pool.Moreover, notice that Uniswap V3 pools (unlike Uniswap V2 pairs) don't have the
sync
mechanism that would allow to easily set reserves in the pool to actual balances of tokens in the pool.This can lead to
DAIamount / 1e12 - USSDamount
values passed tomintRebalancer
orBuyUSSDSellCollateral
to be different than expected. Relevant lines of code: 1, 2.Let's consider a situation where someone has sent some DAI to the pool directly. In this case, it's possible that the price of 1 USSD in the pool is less than 1 DAI (and the threshold is exceeded) but
USSDamount - DAIamount / 1e12
is negative (or smaller than expected) because the DAI previously sent directly to the pool didn't influence the price.If that's the case, then call to
rebalance()
will revert becasue of underflow inBuyUSSDSellCollateral((USSDamount - DAIamount / 1e12) / 2);
. This will DoS therebalance()
functionality of USSD, and likely will cause USSD to depeg without option to use collateral in the USSD contract to recover the peg. This will, in turn, cause USSD to lose its value because not many people are comfortable with holding depegged stablecoins.Even if
USSDamount - DAIamount / 1e12
is not negative, just smaller than expected, then too little USSD will be bought to fully recover the peg.Now let's consider a situation, in which the price of 1 USSD more than 1 DAI but someone has sent DAI directly to the pool before. This will cause minting more USSD than expected in this line and overshoot the 1 DAI price for 1 USSD. This situation is intended to be prevented as we can read in the whitepaper or in comments in this line:
Impact
If someone sends DAI to the pool directly (by transfer, not by swap), then USSD will depeg because of
USSDRebalancer.rebalance()
rebalancing too little or even reverting.Another possibility is that selling USSD for DAI in
rebalance
will overshoot and create unwelcome arbitrage opportunities.Code Snippet
https://github.com/sherlock-audit/2023-05-USSD/blob/6d7a9fdfb1f1ed838632c25b6e1b01748d0bafda/ussd-contracts/contracts/USSDRebalancer.sol#L83-L85
https://github.com/sherlock-audit/2023-05-USSD/blob/6d7a9fdfb1f1ed838632c25b6e1b01748d0bafda/ussd-contracts/contracts/USSDRebalancer.sol#L94
https://github.com/sherlock-audit/2023-05-USSD/blob/6d7a9fdfb1f1ed838632c25b6e1b01748d0bafda/ussd-contracts/contracts/USSDRebalancer.sol#L97
https://github.com/sherlock-audit/2023-05-USSD/blob/6d7a9fdfb1f1ed838632c25b6e1b01748d0bafda/ussd-contracts/contracts/USSDRebalancer.sol#L104
Tool used
Manual Review
Recommendation
Don't rely on
USSD.balanceOf(uniPool)
,DAI.balanceOf(uniPool)
. Figure out how to achieve intended behavior usingsqrtPriceX96
only.Duplicate of #451
The text was updated successfully, but these errors were encountered: