code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Impact

The KUMASwap.buyBond mints KBCTokens clone token for every KUMABondToken whose bondFaceValue is greater than realizedBondValue. If bondFaceValue is not greater than realizedBondValue a simple KUMABondToken transfer is done to the caller.

    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 a clone token for a KUMABondToken id as well as receive the KUMABondToken token 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 initially 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.
Due to market conditions, the bondFaceValue now becomes less than or equal to realizedBondValue.
Attacker invokes the buyBond function again with token id 1 as input. This transfers the KUMABondToken with id 1 to the attacker.

Hence now attacker has both the KUMABondToken and the cloned KBCToken.

Tools Used

Manual review

Recommended Mitigation Steps

Consider tracking the already minted KBCToken tokens so that the use of same KUMABondToken id can be prevented.

c4-judge commented 1 year ago

GalloDaSballo marked the issue as primary issue

GalloDaSballo commented 1 year ago

Looks invalid per the following test

Added to KBCToken.t.sol

function test_issueBondABC() public {
        vm.expectEmit(false, false, false, true);
        emit CloneBondIssued(
            1,
            IKBCToken.CloneBond({
                parentId: 1,
                issuance: _KIBToken.getPreviousEpochTimestamp(),
                coupon: _ORACLE_RATE,
                principal: _KUMASwap.getBondBaseValue(1).rayMul(_KIBToken.getUpdatedCumulativeYield()).rayToWad()
            })
            );
        _KUMASwap.buyBond(1);
        _KUMASwap.buyBond(1);

        assertEq(_KBCToken.getBond(1).parentId, 1);
        assertEq(_KBCToken.getBond(1).coupon, _ORACLE_RATE);
        assertEq(_KBCToken.ownerOf(1), address(this));
        assertEq(_KBCToken.balanceOf(address(this)), 1);
        assertEq(_KBCToken.getTokenIdCounter(), 1);
    }

See Terminal

[FAIL. Reason: INVALID_TOKEN_ID()] test_issueBondABC() (gas: 470834)
Traces:
  [427634] Owner::test_issueBondABC() 
    ├─ [0] VM::expectEmit(false, false, false, true) 
    │   └─ ← ()
    ├─ [7433] KIBToken::getPreviousEpochTimestamp() [staticcall]
    │   ├─ [2538] KIBToken::getPreviousEpochTimestamp() [delegatecall]
    │   │   └─ ← 977630400
    │   └─ ← 977630400
    ├─ [16068] KIBToken::getUpdatedCumulativeYield() [staticcall]
    │   ├─ [15673] KIBToken::getUpdatedCumulativeYield() [delegatecall]
    │   │   └─ ← 1029999999999999999980736167
    │   └─ ← 1029999999999999999980736167
    ├─ [7381] KUMASwap::getBondBaseValue(1) [staticcall]
    │   ├─ [2483] KUMASwap::getBondBaseValue(1) [delegatecall]
    │   │   └─ ← 10000000000000000000000000000
    │   └─ ← 10000000000000000000000000000
    ├─ emit CloneBondIssued(ghostId: 1, cloneBond: (1, 977630400, 1000000000937303470807876290, 10300000000000000000))
    ├─ [373125] KUMASwap::buyBond(1) 
    │   ├─ [372730] KUMASwap::buyBond(1) [delegatecall]
    │   │   ├─ [7237] ERC1967Proxy::getKUMABondToken() [staticcall]
    │   │   │   ├─ [2342] KUMAAddressProvider::getKUMABondToken() [delegatecall]
    │   │   │   │   └─ ← KUMABondToken: [0x2a07706473244BC757E10F2a9E86fB532828afe3]
    │   │   │   └─ ← KUMABondToken: [0x2a07706473244BC757E10F2a9E86fB532828afe3]
    │   │   ├─ [14357] KUMABondToken::getBond(1) [staticcall]
    │   │   │   └─ ← (21801507043498557184816715737692617808718952153210948119388872952571265810432, 38593573100551470263584073598280321395493192097713692912252663682477911441408, 31360334495951980292004516990666767453509112676581078859397911793729059946496, 31806780860140474701158521608374019121926365111554892484105154835669741207552)
    │   │   ├─ [2931] ERC1967Proxy::getKIBToken(0x75e14f6b35e6405559895d3db64e15d926e39390d945ecd07a7288f0d529dbd1) [staticcall]
    │   │   │   ├─ [2533] KUMAAddressProvider::getKIBToken(0x75e14f6b35e6405559895d3db64e15d926e39390d945ecd07a7288f0d529dbd1) [delegatecall]
    │   │   │   │   └─ ← KIBToken: [0x03A6a84cD762D9707A21605b548aaaB891562aAb]
    │   │   │   └─ ← KIBToken: [0x03A6a84cD762D9707A21605b548aaaB891562aAb]
    │   │   ├─ [931] ERC1967Proxy::getKIBToken(0x75e14f6b35e6405559895d3db64e15d926e39390d945ecd07a7288f0d529dbd1) [staticcall]
    │   │   │   ├─ [533] KUMAAddressProvider::getKIBToken(0x75e14f6b35e6405559895d3db64e15d926e39390d945ecd07a7288f0d529dbd1) [delegatecall]
    │   │   │   │   └─ ← KIBToken: [0x03A6a84cD762D9707A21605b548aaaB891562aAb]
    │   │   │   └─ ← KIBToken: [0x03A6a84cD762D9707A21605b548aaaB891562aAb]
    │   │   ├─ [933] KIBToken::getPreviousEpochTimestamp() [staticcall]
    │   │   │   ├─ [538] KIBToken::getPreviousEpochTimestamp() [delegatecall]
    │   │   │   │   └─ ← 977630400
    │   │   │   └─ ← 977630400
    │   │   ├─ [10068] KIBToken::getUpdatedCumulativeYield() [staticcall]
    │   │   │   ├─ [9673] KIBToken::getUpdatedCumulativeYield() [delegatecall]
    │   │   │   │   └─ ← 1029999999999999999980736167
    │   │   │   └─ ← 1029999999999999999980736167
    │   │   ├─ [2805] ERC1967Proxy::getKBCToken() [staticcall]
    │   │   │   ├─ [2410] KUMAAddressProvider::getKBCToken() [delegatecall]
    │   │   │   │   └─ ← KBCToken: [0x1d1499e622D69689cdf9004d05Ec547d650Ff211]
    │   │   │   └─ ← KBCToken: [0x1d1499e622D69689cdf9004d05Ec547d650Ff211]
    │   │   ├─ [933] KIBToken::getPreviousEpochTimestamp() [staticcall]
    │   │   │   ├─ [538] KIBToken::getPreviousEpochTimestamp() [delegatecall]
    │   │   │   │   └─ ← 977630400
    │   │   │   └─ ← 977630400
    │   │   ├─ [733] KIBToken::getYield() [staticcall]
    │   │   │   ├─ [338] KIBToken::getYield() [delegatecall]
    │   │   │   │   └─ ← 1000000000937303470807876290
    │   │   │   └─ ← 1000000000937303470807876290
    │   │   ├─ [177895] KBCToken::issueBond(Owner: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496], (1, 977630400, 1000000000937303470807876290, 10300000000000000000)) 
    │   │   │   ├─ [172976] KBCToken::issueBond(Owner: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496], (1, 977630400, 1000000000937303470807876290, 10300000000000000000)) [delegatecall]
    │   │   │   │   ├─ [737] ERC1967Proxy::getKUMABondToken() [staticcall]
    │   │   │   │   │   ├─ [342] KUMAAddressProvider::getKUMABondToken() [delegatecall]
    │   │   │   │   │   │   └─ ← KUMABondToken: [0x2a07706473244BC757E10F2a9E86fB532828afe3]
    │   │   │   │   │   └─ ← KUMABondToken: [0x2a07706473244BC757E10F2a9E86fB532828afe3]
    │   │   │   │   ├─ [2357] KUMABondToken::getBond(1) [staticcall]
    │   │   │   │   │   └─ ← (21801507043498557184816715737692617808718952153210948119388872952571265810432, 38593573100551470263584073598280321395493192097713692912252663682477911441408, 31360334495951980292004516990666767453509112676581078859397911793729059946496, 31806780860140474701158521608374019121926365111554892484105154835669741207552)
    │   │   │   │   ├─ [2909] ERC1967Proxy::getKUMASwap(0x75e14f6b35e6405559895d3db64e15d926e39390d945ecd07a7288f0d529dbd1) [staticcall]
    │   │   │   │   │   ├─ [2511] KUMAAddressProvider::getKUMASwap(0x75e14f6b35e6405559895d3db64e15d926e39390d945ecd07a7288f0d529dbd1) [delegatecall]
    │   │   │   │   │   │   └─ ← KUMASwap: [0x212224D2F2d262cd093eE13240ca4873fcCBbA3C]
    │   │   │   │   │   └─ ← KUMASwap: [0x212224D2F2d262cd093eE13240ca4873fcCBbA3C]
    │   │   │   │   ├─ emit Transfer(from: 0x0000000000000000000000000000000000000000, to: Owner: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496], tokenId: 1)
    │   │   │   │   ├─ [788] Owner::onERC721Received(KUMASwap: [0x212224D2F2d262cd093eE13240ca4873fcCBbA3C], 0x0000000000000000000000000000000000000000, 1, 0x) 
    │   │   │   │   │   └─ ← 0x150b7a02
    │   │   │   │   ├─ emit CloneBondIssued(ghostId: 1, cloneBond: (1, 977630400, 1000000000937303470807876290, 10300000000000000000))
    │   │   │   │   └─ ← 1
    │   │   │   └─ ← 1
    │   │   ├─ emit MinCouponUpdated(oldMinCoupon: 1000000001547125957863212449, newMinCoupon: 1000000000000000000000000000)
    │   │   ├─ [85976] KIBToken::burn(Owner: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496], 10300000000000000000) 
    │   │   │   ├─ [85578] KIBToken::burn(Owner: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496], 10300000000000000000) [delegatecall]
    │   │   │   │   ├─ [2772] ERC1967Proxy::getAccessController() [staticcall]
    │   │   │   │   │   ├─ [2377] KUMAAddressProvider::getAccessController() [delegatecall]
    │   │   │   │   │   │   └─ ← KUMAAccessController: [0x2e234DAe75C793f67A35089C9d99245E1C58470b]
    │   │   │   │   │   └─ ← KUMAAccessController: [0x2e234DAe75C793f67A35089C9d99245E1C58470b]
    │   │   │   │   ├─ [2643] KUMAAccessController::hasRole(0x84c569f1c5bb132044345bdca30291732479c12f6cc24d0a4df8971c7e445a61, KUMASwap: [0x212224D2F2d262cd093eE13240ca4873fcCBbA3C]) [staticcall]
    │   │   │   │   │   └─ ← true
    │   │   │   │   ├─ emit PreviousEpochCumulativeYieldUpdated(oldCumulativeYield: 1000000000000000000000000000, newCumulativeYield: 1029999999999999999980736167)
    │   │   │   │   ├─ emit CumulativeYieldUpdated(oldCumulativeYield: 1000000000000000000000000000, newCumulativeYield: 1029999999999999999980736167)
    │   │   │   │   ├─ [909] ERC1967Proxy::getKUMASwap(0x75e14f6b35e6405559895d3db64e15d926e39390d945ecd07a7288f0d529dbd1) [staticcall]
    │   │   │   │   │   ├─ [511] KUMAAddressProvider::getKUMASwap(0x75e14f6b35e6405559895d3db64e15d926e39390d945ecd07a7288f0d529dbd1) [delegatecall]
    │   │   │   │   │   │   └─ ← KUMASwap: [0x212224D2F2d262cd093eE13240ca4873fcCBbA3C]
    │   │   │   │   │   └─ ← KUMASwap: [0x212224D2F2d262cd093eE13240ca4873fcCBbA3C]
    │   │   │   │   ├─ [911] KUMASwap::isExpired() [staticcall]
    │   │   │   │   │   ├─ [516] KUMASwap::isExpired() [delegatecall]
    │   │   │   │   │   │   └─ ← false
    │   │   │   │   │   └─ ← false
    │   │   │   │   ├─ [783] KUMASwap::isDeprecated() [staticcall]
    │   │   │   │   │   ├─ [388] KUMASwap::isDeprecated() [delegatecall]
    │   │   │   │   │   │   └─ ← false
    │   │   │   │   │   └─ ← false
    │   │   │   │   ├─ [2728] ERC1967Proxy::getRateFeed() [staticcall]
    │   │   │   │   │   ├─ [2333] KUMAAddressProvider::getRateFeed() [delegatecall]
    │   │   │   │   │   │   └─ ← RateFeed: [0x13aa49bAc059d709dd0a18D6bb63290076a702D7]
    │   │   │   │   │   └─ ← RateFeed: [0x13aa49bAc059d709dd0a18D6bb63290076a702D7]
    │   │   │   │   ├─ [17914] RateFeed::getRate(0x75e14f6b35e6405559895d3db64e15d926e39390d945ecd07a7288f0d529dbd1) [staticcall]
    │   │   │   │   │   ├─ [13016] MCAGRateFeed::getRate(0x75e14f6b35e6405559895d3db64e15d926e39390d945ecd07a7288f0d529dbd1) [delegatecall]
    │   │   │   │   │   │   ├─ [6663] MCAGAggregator::latestRoundData() [staticcall]
    │   │   │   │   │   │   │   └─ ← 2, 1000000000937303470807876290, 0, 946094400, 2
    │   │   │   │   │   │   ├─ [202] MCAGAggregator::decimals() [staticcall]
    │   │   │   │   │   │   │   └─ ← 27
    │   │   │   │   │   │   └─ ← 1000000000937303470807876290
    │   │   │   │   │   └─ ← 1000000000937303470807876290
    │   │   │   │   ├─ [754] KUMASwap::getMinCoupon() [staticcall]
    │   │   │   │   │   ├─ [359] KUMASwap::getMinCoupon() [delegatecall]
    │   │   │   │   │   │   └─ ← 1000000000000000000000000000
    │   │   │   │   │   └─ ← 1000000000000000000000000000
    │   │   │   │   ├─ emit YieldUpdated(oldYield: 1000000000937303470807876290, newYield: 1000000000000000000000000000)
    │   │   │   │   ├─ [4457] KIBToken::balanceOf(Owner: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496]) [staticcall]
    │   │   │   │   │   ├─ [4059] KIBToken::balanceOf(Owner: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496]) [delegatecall]
    │   │   │   │   │   │   └─ ← 10300000000000000000
    │   │   │   │   │   └─ ← 10300000000000000000
    │   │   │   │   ├─ emit Transfer(from: Owner: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496], to: 0x0000000000000000000000000000000000000000, value: 10300000000000000000)
    │   │   │   │   └─ ← ()
    │   │   │   └─ ← ()
    │   │   ├─ emit BondBought(tokenId: 1, KIBTokenBurned: 10300000000000000000, buyer: Owner: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496])
    │   │   └─ ← ()
    │   └─ ← ()
    ├─ [6481] KUMASwap::buyBond(1) 
    │   ├─ [6082] KUMASwap::buyBond(1) [delegatecall]
    │   │   ├─ [737] ERC1967Proxy::getKUMABondToken() [staticcall]
    │   │   │   ├─ [342] KUMAAddressProvider::getKUMABondToken() [delegatecall]
    │   │   │   │   └─ ← KUMABondToken: [0x2a07706473244BC757E10F2a9E86fB532828afe3]
    │   │   │   └─ ← KUMABondToken: [0x2a07706473244BC757E10F2a9E86fB532828afe3]
    │   │   ├─ [2357] KUMABondToken::getBond(1) [staticcall]
    │   │   │   └─ ← (21801507043498557184816715737692617808718952153210948119388872952571265810432, 38593573100551470263584073598280321395493192097713692912252663682477911441408, 31360334495951980292004516990666767453509112676581078859397911793729059946496, 31806780860140474701158521608374019121926365111554892484105154835669741207552)
    │   │   └─ ← "INVALID_TOKEN_ID()"
    │   └─ ← "INVALID_TOKEN_ID()"
    └─ ← "INVALID_TOKEN_ID()"

Test result: FAILED. 0 passed; 1 failed; finished in 9.47ms

Failing tests:
Encountered 1 failing test in test/kuma-protocol/KBCToken.t.sol:KBCTokenTest
[FAIL. Reason: INVALID_TOKEN_ID()] test_issueBondABC() (gas: 470834)

GalloDaSballo commented 1 year ago

@akshaysrivastav lmk if you have a specific test case that demonstrates the attack

c4-sponsor commented 1 year ago

m19 marked the issue as sponsor disputed

m19 commented 1 year ago

We also dispute the validity of this, the bond is removed from the bondReserve when a clone is issued so the attack is not possible.

akshaysrivastav commented 1 year ago

My bad, somehow I overlooked the _bondReserve.remove(tokenId); statement. The reported attack scenario is invalid.