code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-02-kuma/blob/main/src/kuma-protocol/KUMASwap.sol#L177

Vulnerability details

Impact

The KUMASwap.buyBond mints KBCTokens clone token for every KUMABondToken whose bondFaceValue is greater than realizedBondValue.

    function buyBond(uint256 tokenId) external override whenNotPaused whenNotDeprecated {
        IKUMAAddressProvider KUMAAddressProvider = _KUMAAddressProvider;
        IKUMABondToken KUMABondToken = IKUMABondToken(KUMAAddressProvider.getKUMABondToken());
        IKUMABondToken.Bond memory bond = KUMABondToken.getBond(tokenId);

        if (!_bondReserve.contains(tokenId)) {
            revert Errors.INVALID_TOKEN_ID();
        }

        bool isBondExpired = _expiredBonds.contains(tokenId);

        if (_expiredBonds.length() > 0 && !isBondExpired) {
            revert Errors.EXPIRED_BONDS_MUST_BE_BOUGHT_FIRST();
        }

        if (_couponInventory[bond.coupon] == 1) {
            _coupons.remove(bond.coupon);
        }

        _couponInventory[bond.coupon]--;
        _bondReserve.remove(tokenId);

        if (isBondExpired) {
            _expiredBonds.remove(tokenId);
        }

        IKIBToken KIBToken = IKIBToken(KUMAAddressProvider.getKIBToken(_riskCategory));

        uint256 bondFaceValue = _getBondValue(bond.issuance, bond.term, bond.coupon, bond.principal);
        uint256 realizedBondValue = _bondBaseValue[tokenId].rayMul(KIBToken.getUpdatedCumulativeYield()).rayToWad();

        bool requireClone = bondFaceValue > realizedBondValue;

        if (requireClone) {
            _cloneBonds[tokenId] = IKBCToken(KUMAAddressProvider.getKBCToken()).issueBond(
                msg.sender,
                IKBCToken.CloneBond({
                    parentId: tokenId,
                    issuance: KIBToken.getPreviousEpochTimestamp(),
                    coupon: KIBToken.getYield(),
                    principal: realizedBondValue
                })
            );
        }

        _updateMinCoupon();

        KIBToken.burn(msg.sender, realizedBondValue);

        if (!requireClone) {
            KUMABondToken.safeTransferFrom(address(this), msg.sender, tokenId);
        }

        emit BondBought(tokenId, realizedBondValue, msg.sender);
    }

However the function does not track whether a clone token is already minted for a particular KUMABondToken id. This can be misused by an attacker to mint multiple clone tokens for same KUMABondToken id by invoking the buyBond function multiple times.

Please note that every buyBond call cost the attacker realizedBondValue KIBToken amount.

Proof of Concept

Consider this scenario:

KUMABondToken with id 1 was sold to the KUMASwap contract.
Assume that for this token id the bondFaceValue is greater than realizedBondValue.
Attacker invokes the buyBond function with token id 1 as input. This mints a clone KBCToken token to the attacker.
Attacker invokes the buyBond functionn again with token id 1 as input. This again mints another clone KBCToken token to the attacker.

Tools Used

Manual review

Recommended Mitigation Steps

Consider tracking the already minted KBCToken tokens so that their double minting can be prevented.

c4-judge commented 1 year ago

GalloDaSballo marked the issue as duplicate of #33