rvierdiiev - TieredFixedBountyV1.setPayoutScheduleFixed will fail in case issuer wants to make less amount of tiers

[github-actions]

rvierdiiev

medium

TieredFixedBountyV1.setPayoutScheduleFixed will fail in case issuer wants to make less amount of tiers

Summary

TieredFixedBountyV1.setPayoutScheduleFixed will fail in case issuer wants to make less amount of tiers

Vulnerability Detail

TieredFixedBountyV1.setPayoutScheduleFixed allows to provide new _payoutSchedule to the bounty. This should update values inside tierWinners, invoiceComplete, supportingDocumentsComplete arrays.
https://github.com/sherlock-audit/2023-02-openq/blob/main/contracts/Bounty/Implementations/TieredFixedBountyV1.sol#L138-L171

    function setPayoutScheduleFixed(
        uint256[] calldata _payoutSchedule,
        address _payoutTokenAddress
    ) external onlyOpenQ {
        require(
            bountyType == OpenQDefinitions.TIERED_FIXED,
            Errors.NOT_A_FIXED_TIERED_BOUNTY
        );
        payoutSchedule = _payoutSchedule;
        payoutTokenAddress = _payoutTokenAddress;


        // Resize metadata arrays and copy current members to new array
        // NOTE: If resizing to fewer tiers than previously, the final indexes will be removed
        string[] memory newTierWinners = new string[](payoutSchedule.length);
        bool[] memory newInvoiceComplete = new bool[](payoutSchedule.length);
        bool[] memory newSupportingDocumentsCompleted = new bool[](
            payoutSchedule.length
        );


        for (uint256 i = 0; i < tierWinners.length; i++) {
            newTierWinners[i] = tierWinners[i];
        }
        tierWinners = newTierWinners;


        for (uint256 i = 0; i < invoiceComplete.length; i++) {
            newInvoiceComplete[i] = invoiceComplete[i];
        }
        invoiceComplete = newInvoiceComplete;


        for (uint256 i = 0; i < supportingDocumentsComplete.length; i++) {
            newSupportingDocumentsCompleted[i] = supportingDocumentsComplete[i];
        }
        supportingDocumentsComplete = newSupportingDocumentsCompleted;
    }

In case if new payoutSchedule is less than previous value(that was set on init), then this function will revert with array out of bounds error, because all loops are iterating using length of previous `payoutSchedule.

Add this test to OpenQ.test.js

it.only('revert when decrease tiers count', async () => {
      // ARRANGE
      await openQProxy.mintBounty(
        Constants.bountyId,
        Constants.organization,
        tieredFixedBountyInitOperation
      )
const bountyAddress = await openQProxy.bountyIdToAddress(Constants.bountyId)
      const bounty = await TieredFixedBountyV1.attach(bountyAddress)

      await expect(
        openQProxy.setPayoutScheduleFixed(
          Constants.bountyId,
          [70, 20, 10],
          mockDai.address
        )
      )
        .to.emit(openQProxy, 'PayoutScheduleSet')
        .withArgs(
          bountyAddress,
          mockDai.address,
          [70, 20, 10],
          3,
          [],
          Constants.VERSION_1
        )

        //it reverts as array is less
        await openQProxy.setPayoutScheduleFixed(
          Constants.bountyId,
          [70, 20],
          mockDai.address
        )
    })

You will receive such error: Error: VM Exception while processing transaction: reverted with panic code 0x32 (Array accessed at an out-of-bounds or negative index)

Also same issue exists inside TieredPercentageBountyV1.setPayoutSchedule and i am not sure if i should create it as separate report or use only one.

Impact

It's not possible to decrease amount of tiers.

Code Snippet

Provided above

Tool used

Manual Review

Recommendation

Use _payoutSchedule.length to iterate through new arrays.

Duplicate of #244