DOS of removeAsset() when asset.locations.length > 1

Summary

The removeAssets function in the OlympusTreasury contract has a critical bug that causes it to revert if the asset to be removed has more than one location.

Vulnerability Detail

https://github.com/sherlock-audit/2023-11-olympus/blob/main/bophades/src/modules/TRSRY/OlympusTreasury.sol#L450-L490

On line 471, all locations are iterated, and the pop() function is used to remove them from the array. The problem lies in line 472, where, at each iteration, the element at position len-1 is assigned to the element at position i. Len is a cached value of the array length before the start of the for loop, so its value does not change when the array length decreases after the pop() operation. This leads to accessing an element that no longer exists, resulting in a revert with an "Out of bound" error. Line 472 is redundant and can be removed to fix the issue. Simple POC that demonstrate the revert:

    function testCorrectness_removeAsset_AssetConfigured() public {
        address[] memory locations = new address[](5);
        locations[0] = address(1);
        locations[1] = address(2);
        locations[2] = address(3);
        locations[3] = address(4);
        locations[4] = address(5);

        // Add an asset
        vm.prank(godmode);
        TRSRY.addAsset(address(reserve), locations);

        // Remove the asset
        vm.prank(godmode);
        TRSRY.removeAsset(address(reserve));

        // Verify asset data
        TRSRYv1_1.Asset memory asset = TRSRY.getAssetData(address(reserve));
        assertEq(asset.approved, false);
        assertEq(asset.lastBalance, 0);
        assertEq(asset.locations.length, 0);

        // Verify asset list
        address[] memory assets = TRSRY.getAssets();
        assertEq(assets.length, 0);
    }

Impact

Broken core functionality. User would not be able to remove an asset with multiple locations using removeAsset. There is multiple steps workaround with other functions, so it should be Medium severity.

Code Snippet

https://github.com/sherlock-audit/2023-11-olympus/blob/main/bophades/src/modules/TRSRY/OlympusTreasury.sol#L450-L490

Tool used

Manual Review

Recommendation

Remove line 472.

Duplicate of #151

sherlock-audit / 2023-11-olympus-judging

ge6a - DOS of removeAsset() when asset.locations.length > 1 #202