0x52 - Target raises can be highly damaging for dutch auctions with multiple components

[sherlock-admin]

0x52

medium

Target raises can be highly damaging for dutch auctions with multiple components

Summary

Multi-component dutch auctions are fundamentally incompatible with target raises and will lead to inefficient pricing causing loss to set token.

Vulnerability Detail

The AuctionRebalanceModuleV1 allows targets to be increased when all component targets have been met and there is still excess quote token. When combined with multiple components, it his highly likely that these target raises will lead to inefficient pricing which will cause loss to the set token.

Consider the following a set token has the following composition that has target raises enabled:

40% USDC 30% WBTC 30% WETH

The manager wishes to rebalance the set to the following using USDC as the quote token:

20% USDC 40% WBTC 40% WETH

Assume the WETH portion of the execute within the first hour of the auction. The WBTC on the other hand doesn't execute until 12 hours in. Assume there is excess quote so the target is increased. The issue is that now because of the change in time, the WETH auction is now well above the market price. This buys the WETH for a large loss compared to the market price of WETH.

Impact

Pricing after target raises will likely be heavily skewed from market prices for some components lead to set token losses

Code Snippet

AuctionRebalanceModuleV1.sol#L359-L380

Tool used

Manual Review

Recommendation

Target raises should reset rebalanceStartTime allowing the dutch auction to restart and properly price the assets

[pblivin0x]

Agree, especially since the raising of targets is onlyAllowedBidder, we should reset the pricings.

In the fix, I think we will make it such that the rebalance still ends at the same time.

[bizzyvinci]

Escalate

This is invalid. Let's take a look at why would we would raise target based on the docs

* @dev ACCESS LIMITED: Increases asset targets uniformly when all target units have been met but there is remaining quote asset.

raiseAssetTarget is meant for uniformly raising targets when all target units have been met. Everything else (such as price and ratio) can be resolved by calling startRebalance.

[sherlock-admin2]

Escalate

This is invalid. Let's take a look at why would we would raise target based on the docs

* @dev ACCESS LIMITED: Increases asset targets uniformly when all target units have been met but there is remaining quote asset.

raiseAssetTarget is meant for uniformly raising targets when all target units have been met. Everything else (such as price and ratio) can be resolved by calling startRebalance.

You've created a valid escalation!

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.

[hrishibhat]

@bizzyvinci Could you please elaborate a bit further as to what exactly is invalid about the points raised in the issue?

[bizzyvinci]

First of all, I believe the auction buy and sell is to be used instead of trade module and this was mentioned on the discord channel. And the way it works is that it provides discount over market price to motivate bidder. And this discount increases linearly, exponential or logarithmically.

And an assumption is that the discount will start low e.g 0% till a cap the manager is comfortable with e.g 5%. Whether you're buying or selling, the focus is on discount and that's why price would decrease during sell and price would increase during buy of quoteAsset. The price change is the discount and that's what grows or fall with the mathematical equation.

Assuming manager/operator wants to move from 40% USDC, 30% WBTC, 30% WETH

To 20% USDC, 40% WBTC, 40% WETH using USDC

If after an hour, the ratio are 35% USDC, 30% WBTC, 35% WETH. It's safe to assume that price on WETH was too good but price on WBTC wasn't. The operator can call startRebalance to update this prices.

If the price is indeed good and the market/whale is just crazy for WBTC and it reaches 30% USDC, 30% WBTC, 40% WETH. Then no one can bid on WETH again, because of the following requirements

require(currentUnit != targetUnit, "Target already met");

// Determine whether the component is being sold (sendToken) or bought
isSellAuction = targetNotional < currentNotional;

// Calculate the max quantity of the component to be exchanged. If buying, account for the protocol fees.
maxComponentQty = isSellAuction
    ? currentNotional.sub(targetNotional)
    : targetNotional.sub(currentNotional).preciseDiv(PreciseUnitMath.preciseUnit().sub(protocolFee));

Therefore, the best thing to do is wait till the discount on WBTC is suitable and every WBTC is munched on. Thereby reaching the goal of 20% USDC, 40% WBTC, 40% WETH.

Excess USDC means USDC is greater than 20%. And that also means either WBTC or WETH is less than 40% or both. And it is possible to be unable to bid cause targetUnit of WBTC and WETH has been reached. However, all target unit has not been reached because of USDC. The solution is to call startRebalance again and update targetUnits (and maybe price too is depending on what is considered fair price).

[bizzyvinci]

An example of when to use raiseAssetTargets.

Assuming USDC is quoteAsset but not a component. And we want WETH and WBTC to be 50 and 50 respectively.

If targetUnit and hence the 50:50 is reached and there's still USDC. We could use raiseAssetTarget to uniformly raise the target unit by 10% to 55:55. And that's still a 50% ratio for each component.

[pblivin0x]

I believe this is a valid medium

[bizzyvinci]

I still stand by my escalation because my argument is that the manager could call startRebalance at any point in time during an active auction. With this function startRebalance, they could change price, price curve, and target unit. Therefore no matter the unfavourable price, target unit or ratio, the manager has the option to call startRebalance instead of raiseAssetTarget.

The only reason a manager would call raiseAssetTarget instead of startRebalance is when

• All target unit have been met
• And they want to uniformly raise all the target units

And (though it was not explicitly stated in the docs)

• manager is comfortable with the current price and direction. Cause if they are not, they could call startRebalance to update the price while raising all target units.

[pblivin0x]

I still stand by my escalation because my argument is that the manager could call startRebalance at any point in time during an active auction. With this function startRebalance, they could change price, price curve, and target unit. Therefore no matter the unfavourable price, target unit or ratio, the manager has the option to call startRebalance instead of raiseAssetTarget.

The only reason a manager would call raiseAssetTarget instead of startRebalance is when

• All target unit have been met
• And they want to uniformly raise all the target units

And (though it was not explicitly stated in the docs)

• manager is comfortable with the current price and direction. Cause if they are not, they could call startRebalance to update the price while raising all target units.

Upon further review, I actually change my opinion, and agree with this escalation.

I think that the listed remediation is not satisfactory and that it is on the SetToken manager to decide whether raiseAssetTargets or a fresh startRebalance call is preferable given their price curves.

---

Assume fair market prices of 1850 for ETH and 29000 for WBTC.

Suppose we have a rebalance with the following individual component auctions

• An auction that sells WETH in exchange for DAI. Auction price begins at 2200 and lowers to a price of 1800.
• An auction that buys WBTC with DAI. Auction price begins at 25000 and raises to a price of 30000.

Now suppose that the auctions fill, there are remaining DAI units, and both auction price curves have reached their final price (1800 and 30000).

If asset targets are raised, all auctions now become buy auctions.

If we do not reset rebalanceStartTime when we raise asset targets, we have

• Good: An auction that buys WETH with DAI at a price of 1800. This is below market value, unlikely to get filled, but not a risk for SetToken holders
• Bad: An auction that buys WBTC with DAI at a price of 30000. This is above market value, and a risk to SetToken holders to lose NAV

If we do reset rebalanceStartTime when we raise asset targets, we have

• Bad: An auction that buys WETH with DAI at a price of 2200. This is above market value, and a risk to SetToken holders to lose NAV
• Good: An auction that buys WBTC with DAI at a price of 25000. This is below market value, unlikely to get filled, but not a risk for SetToken holders

---

In conclusion

• I agree with the stated vulnerability - Target raises can be highly damaging for dutch auctions with multiple components
• I disagree with the listed remediation - resetting the rebalanceStartTime does not automatically lead to proper pricing of the component auctions, because sell auctions have flipped to buy auctions on the target raise.
• I agree with the escalation - It is on the SetToken manager to decide whether their auction price curves are appropriate for a nonzero raiseTargetPercentage. If it is not appropriate, then they need to call a fresh startRebalance().

[bizzyvinci]

raiseAssetTargets is for when you are buying all components (those listed for bidding) and you want to sell quoteAsset till it reaches 0 or a specified limit. This was also mentioned in the docs (@pblivin0x could update docs if the wordings are not clear enough for most users)

This helps in reducing tracking error and providing greater granularity in reaching an equilibrium between the excess quote asset and the components to be purchased.

[hrishibhat]

Additional Sponsor comment:

confirming i see this as a low. manager needs to decide between proper calls (setTargetRaisePercentage or startRebalance) based on the AuctionExecutionParams they inputted

[pblivin0x]

This issue's remediation has been removed from https://github.com/IndexCoop/index-protocol/pull/25 pending escalation resolution

[IAm0x52]

I disagree that this is admin's responsibility. The feature is dangerous in this scenario. There is no "safe" parameter that admin can use. Their only option is to not use the feature, which I don't think is a valid. It also has to be considered that the admin can't turn off rebalances after they have been enabled. I still hold this is a valid medium. #44 also provides another way this can be abused. I know that sponsor has commented that donation doesn't work but the donation occurs before the buy which causes the donated balance to be counted and to reflect in the set token balances when it updates the balances of the set token. Both this and #44 have the same root cause (not resetting the price after increase) which is why I didn't escalate that one.

[hrishibhat]

Result: Medium Unique Although the suggested remediation does not solve the problem the issue identified is valid. Sponsor:

I agree with the stated vulnerability - Target raises can be highly damaging for dutch auctions with multiple components

Additional Sponsor comment:

raising asset targets is a legacy feature from the GeneralIndexModule, and as your issue correctly points out, it doesnt really make sense with dutch auctions

i am fine with any issue validity ruling here.

Considering this a valid medium based on the additional sponsor comment and the Lead Watson comment

[sherlock-admin2]

Escalations have been resolved successfully!

Escalation status:

• bizzyvinci: rejected