Maia DAO Ecosystem - BLOS's results

Lines of code

https://github.com/code-423n4/2023-05-maia/blob/54a45beb1428d85999da3f721f923cbf36ee3d35/src/ulysses-amm/UlyssesToken.sol#L72

Vulnerability details

Impact

In function removeAsset, assetId of lastAsset should be assetIndex + 1 instead of assetIndex.

Proof of Concept

function removeAsset(address asset) external nonReentrant onlyOwner {
    // No need to check if index is 0, it will underflow and revert if it is 0
    uint256 assetIndex = assetId[asset] - 1;

    uint256 newAssetsLength = assets.length - 1;

    if (newAssetsLength == 0) revert CannotRemoveLastAsset();

    totalWeights -= weights[assetIndex];

    address lastAsset = assets[newAssetsLength];

    assetId[lastAsset] = assetIndex;
    assets[assetIndex] = lastAsset;
    weights[assetIndex] = weights[newAssetsLength];

    assets.pop();
    weights.pop();
    assetId[asset] = 0;

    emit AssetRemoved(asset);

    updateAssetBalances();

    asset.safeTransfer(msg.sender, asset.balanceOf(address(this)));
}

function addAsset(address asset, uint256 _weight) external nonReentrant onlyOwner {
    if (assetId[asset] != 0) revert AssetAlreadyAdded();
    require(ERC20(asset).decimals() == 18);
    require(_weight > 0);

    assetId[asset] = assets.length + 1;
    assets.push(asset);
    weights.push(_weight);
    totalWeights += _weight;

    emit AssetAdded(asset, _weight);

    updateAssetBalances();
}

When adding or removing asset, id is always index + 1, but this is not followed by assetId[lastAsset] = assetIndex

Tools Used

Manual

Recommended Mitigation Steps

assetId[lastAsset] = assetIndex + 1;

Assessed type

Math