-
Notifications
You must be signed in to change notification settings - Fork 7
0xRobocop - Inconsistency handling of DAI as collateral in the BuyUSSDSellCollateral function #515
Comments
Escalate for 10 USDC This is not a duplicate of #111 This issue points to an inconsistency in handling DAI as a collateral during peg-down recovery scenarios. The contract will try to sell DAI, but DAI does not have a sell path, so the transaction will revert. if (collateralval > amountToBuyLeftUSD) {
// sell a portion of collateral and exit
if (collateral[i].pathsell.length > 0) {
uint256 amountBefore = IERC20Upgradeable(baseAsset).balanceOf(USSD);
uint256 amountToSellUnits = IERC20Upgradeable(collateral[i].token).balanceOf(USSD) * ((amountToBuyLeftUSD * 1e18 / collateralval) / 1e18) / 1e18;
IUSSD(USSD).UniV3SwapInput(collateral[i].pathsell, amountToSellUnits);
amountToBuyLeftUSD -= (IERC20Upgradeable(baseAsset).balanceOf(USSD) - amountBefore);
DAItosell += (IERC20Upgradeable(baseAsset).balanceOf(USSD) - amountBefore);
}
else {
// no need to swap DAI
DAItosell = IERC20Upgradeable(collateral[i].token).balanceOf(USSD) * amountToBuyLeftUSD / collateralval;
}
}
else {
// @audit-issue Not handling the case where this is DAI as is done above.
// sell all or skip (if collateral is too little, 5% treshold)
if (collateralval >= amountToBuyLeftUSD / 20) {
uint256 amountBefore = IERC20Upgradeable(baseAsset).balanceOf(USSD);
// sell all collateral and move to next one
IUSSD(USSD).UniV3SwapInput(collateral[i].pathsell, IERC20Upgradeable(collateral[i].token).balanceOf(USSD));
amountToBuyLeftUSD -= (IERC20Upgradeable(baseAsset).balanceOf(USSD) - amountBefore);
DAItosell += (IERC20Upgradeable(baseAsset).balanceOf(USSD) - amountBefore);
}
} See the inconsistency on the upper |
You've created a valid escalation for 10 USDC! To remove the escalation from consideration: Delete your comment. You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final. |
Result: |
Escalations have been resolved successfully! Escalation status:
|
0xRobocop
high
Inconsistency handling of DAI as collateral in the BuyUSSDSellCollateral function
Summary
DAI is the base asset of the
USSD.sol
contract, when a rebalacing needs to occur during a peg-down recovery, collateral is sold for DAI, which then is used to buy USSD in the DAI / USSD uniswap pool. Hence, when DAI is the collateral, this is not sold because there no existe a path to sell DAI for DAI.Vulnerability Detail
The above behavior is handled when collateral is about to be sold for DAI, see the comment
no need to swap DAI
(link to the code):The problem is in the
else branch
of the first if statementcollateralval > amountToBuyLeftUSD
, which lacks the checkif (collateral[i].pathsell.length > 0)
Impact
A re-balancing on a peg-down recovery will fail if the
collateralval
of DAI is less thanamountToBuyLeftUSD
but greater thanamountToBuyLeftUSD / 20
since the DAI collateral does not have a sell path.Code Snippet
https://github.com/sherlock-audit/2023-05-USSD/blob/6d7a9fdfb1f1ed838632c25b6e1b01748d0bafda/ussd-contracts/contracts/USSDRebalancer.sol#L130-L139
Tool used
Manual Review
Recommendation
Handle the case as is the done on the if branch of
collateralval > amountToBuyLeftUSD
:The text was updated successfully, but these errors were encountered: