0xTheC0der - SetToken can be indefinitely locked by AuctionRebalanceModuleV1

[sherlock-admin]

0xTheC0der

medium

SetToken can be indefinitely locked by AuctionRebalanceModuleV1

Summary

Issuance, redemption, and fee collection of a SetToken can indefinitely locked by AuctionRebalanceModuleV1.

Vulnerability Detail

The AuctionRebalanceModuleV1.unlock(...) method should allow anyone to unlock a previously locked SetToken (locked by AuctionRebalanceModuleV1.startRebalance(...) for a rebalance auction) as soon as the rebalance duration has elapsed or early unlock is possible (targets are met) in order to re-enable issuance, redemption, and fee collection of a SetToken. Moreover, it is worth to mention that a SetToken can only be unlocked by the msg.sender who locked it, i.e. via AuctionRebalanceModuleV1.unlock(...) in this case.

However, as already shown by an existing test case, the early unlock can be blocked although all targets are met by setting raiseTargetPercentage > 0 via AuctionRebalanceModuleV1.setRaiseTargetPercentage(...). Furthermore, the rebalance duration can be extended at any time by the SetToken manager by calling AuctionRebalanceModuleV1.startRebalance(...) again with an unreasonably high rebalance duration (there is no upper limit).

Anyways, there is an even simpler way to cause DoS on every unlock attempt (without even setting raiseTargetPercentage > 0) by just setting the rebalance duration to type(uint256).max which will cause AuctionRebalanceModuleV1._isRebalanceDurationElapsed(...) to revert with 'SafeMath: addition overflow' on every unlock attempt.

Impact

Issuance, redemption, and fee collection of a SetToken can be indefinitely locked by AuctionRebalanceModuleV1 by simply blocking or causing DoS of AuctionRebalanceModuleV1.unlock(...).
This behaviour should not be possible and is not suggested by the AuctionRebalanceModuleV1.unlock(...) method which is accessible by anyone and should allow to unlock a SetToken at least after a reasonable amount of time.

Code Snippet

The following PoC modifies existing test cases to demonstrate DoS on all unlock attempts. 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..e3d0e4f 100644
--- a/test/protocol/modules/v1/auctionRebalanceModuleV1.spec.ts
+++ b/test/protocol/modules/v1/auctionRebalanceModuleV1.spec.ts
@@ -1,5 +1,5 @@
 import "module-alias/register";
-import { BigNumber } from "ethers";
+import { BigNumber, constants } from "ethers";
 
 import { Address, AuctionExecutionParams, StreamingFeeState } from "@utils/types";
 import { Account } from "@utils/test/types";
@@ -1552,13 +1552,14 @@ describe("AuctionRebalanceModuleV1", () => {
       });
     });
 
-    describe("#unlock", async () => {
+    describe.only("#unlock", async () => {
       let subjectIncreaseTime: BigNumber;
 
       beforeEach(async () => {
         subjectIncreaseTime = defaultDuration.add(1);
         subjectSetToken = indexWithQuoteAsset;
         subjectCaller = await getRandomAccount();
+        defaultDuration = constants.MaxUint256; // will cause DoS of all unlock attempts with "SafeMath: addition overflow"
       });
 
       async function subject(): Promise<ContractTransaction> {

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). See issue "New auction rebalance can be started before previous one concluded or duration elapsed".

Alternative: In case the described behaviour is intended, the onlyManagerAndValidSet(_setToken) modifier should be added to the unlock(...) method in order to make it clear that the SetToken manager is effectively in total control of the lock/unlock mechanism.

[MarioPoneder]

Escalate

This is a duplicate of #38.
Also contains reasoning with raiseTargetPercentage and additional ways to block unlock.

[sherlock-admin2]

Escalate

This is a duplicate of #38.
Also contains reasoning with raiseTargetPercentage and additional ways to block unlock.

The escalation could not be created because you are not exceeding the escalation threshold.

You can view the required number of additional valid issues/judging contest payouts in your Profile page,
in the Sherlock webapp.