Flawed clearing of asset locations array will cause removeAsset to revert
Summary
Incorrect clearing of asset locations array
Vulnerability Detail
In removeAsset when removing the locations, the len-1 index is accessed with the initial length even after popping the array. In case the number of locations is more than 1, it will revert due to out of bound access
len = asset.locations.length;
for (uint256 i; i < len; ) {
asset.locations[i] = asset.locations[len - 1];
asset.locations.pop();
unchecked {
++i;
}
}
Impact
Assets with more than 2 locations cannot be removed
hash
medium
Flawed clearing of asset locations array will cause
removeAsset
to revertSummary
Incorrect clearing of asset locations array
Vulnerability Detail
In
removeAsset
when removing the locations, thelen-1
index is accessed with the initial length even after popping the array. In case the number of locations is more than 1, it will revert due to out of bound accessImpact
Assets with more than 2 locations cannot be removed
Code Snippet
https://github.com/sherlock-audit/2023-11-olympus/blob/main/bophades/src/modules/TRSRY/OlympusTreasury.sol#L470-L477
Tool used
Manual Review
Recommendation
Since the assetData is being cleared in the end, the loop can be fully avoided
Duplicate of #151