Drynooo - location may be added repeatedly

[sherlock-admin2]

Drynooo

medium

location may be added repeatedly

Summary

In the addAsset function of the OlympusTreasury contract, locations are configured, but locations_ are not checked to see if they contain duplicate data. The same location may be added repeatedly and be included in the asset balance repeatedly.

Vulnerability Detail

locations represents the address array held by the vault that owns this asset. If the array contains the same address multiple times, it will result in repeated balance statistics.

There is a check for duplication in the addAssetLocation function, but there is no check in the addAsset function.

OlympusTreasury.sol#L498C1-L519C6

    function addAssetLocation(address asset_, address location_) external override permissioned {
        Asset storage asset = assetData[asset_];

        // Check that asset is approved
        if (!asset.approved) revert TRSRY_AssetNotApproved(asset_);

        // Check that the location is not the zero address
        if (location_ == address(0)) revert TRSRY_InvalidParams(1, abi.encode(location_));

        // Check that location is not already added
        uint256 len = asset.locations.length;
        for (uint256 i; i < len; ) {
            if (asset.locations[i] == location_)
                revert TRSRY_InvalidParams(1, abi.encode(location_));
            unchecked {
                ++i;
            }
        }

        // Add location to array
        asset.locations.push(location_);
    }

OlympusTreasury.sol#L415C1-L444C6

    function addAsset(
        address asset_,
        address[] calldata locations_
    ) external override permissioned {
        Asset storage asset = assetData[asset_];

        // Ensure asset is not already added
        if (asset.approved) revert TRSRY_AssetAlreadyApproved(asset_);

        // Check that asset is a contract
        if (asset_.code.length == 0) revert TRSRY_AssetNotContract(asset_);

        // Set asset as approved and add to array
        asset.approved = true;
        assets.push(asset_);

        // Validate balance locations and store
        uint256 len = locations_.length;
        for (uint256 i; i < len; ) {
            if (locations_[i] == address(0))
                revert TRSRY_InvalidParams(1, abi.encode(locations_[i]));
            asset.locations.push(locations_[i]);
            unchecked {
                ++i;
            }
        }

        // Initialize cache with current value
        (asset.lastBalance, asset.updatedAt) = _getCurrentBalance(asset_);
    }

Impact

Possible double-counting of asset balances, resulting in a large display of assets held in the vault. It will ultimately affect the exchange price of OHM.

Code Snippet

OlympusTreasury.sol#L415C1-L444C6

    function addAsset(
        address asset_,
        address[] calldata locations_
    ) external override permissioned {
        Asset storage asset = assetData[asset_];

        // Ensure asset is not already added
        if (asset.approved) revert TRSRY_AssetAlreadyApproved(asset_);

        // Check that asset is a contract
        if (asset_.code.length == 0) revert TRSRY_AssetNotContract(asset_);

        // Set asset as approved and add to array
        asset.approved = true;
        assets.push(asset_);

        // Validate balance locations and store
        uint256 len = locations_.length;
        for (uint256 i; i < len; ) {
            if (locations_[i] == address(0))
                revert TRSRY_InvalidParams(1, abi.encode(locations_[i]));
            asset.locations.push(locations_[i]);
            unchecked {
                ++i;
            }
        }

        // Initialize cache with current value
        (asset.lastBalance, asset.updatedAt) = _getCurrentBalance(asset_);
    }

Tool used

Manual Review

Recommendation

It is recommended to check whether there is duplicate data in locations.

Duplicate of #78