code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-02-kuma/blob/3f3d2269fcb3437a9f00ffdd67b5029487435b95/src/kuma-protocol/KUMASwap.sol#L115-L171

Vulnerability details

Impact

Selling bonds with coupons that are already accounted will fail unexpectedly

Proof of Concept

https://github.com/code-423n4/2023-02-kuma/blob/3f3d2269fcb3437a9f00ffdd67b5029487435b95/src/kuma-protocol/KUMASwap.sol#L116-L118

    if (_coupons.length() == _maxCoupons) {
        revert Errors.MAX_COUPONS_REACHED();
    }

The above lines will cause ALL bonds sales to revert when _coupons.length has reached _maxCoupons. Since bonds may share the same coupon, the swap should continue to accept bonds with a coupon that already exist in the _coupons set.

Tools Used

Manual Review

Recommended Mitigation Steps

sellBond should only revert if the max length has been reached and bond.coupon doesn't already exist:

-   if (_coupons.length() == _maxCoupons) {
+   if (_coupons.length() == _maxCoupons && !_coupons.contains(bond.coupon)) {
        revert Errors.MAX_COUPONS_REACHED();
    }

c4-sponsor commented 1 year ago

m19 marked the issue as sponsor confirmed

m19 commented 1 year ago

Even though this scenario is unlikely to ever happen we have confirmed this issue in a test:

    function test_sellBond_WithExistingCouponWhenMaxCouponsReached() external {
        _KUMASwap.sellBond(1);
        IKUMABondToken.Bond memory bond_ = _bond;

        for (uint256 i; i < 364; i++) {
            bond_.coupon = bond_.coupon + 1;
            _KUMABondToken.issueBond(address(this), bond_);
            _KUMASwap.sellBond(i + 2);
        }

        _KUMABondToken.issueBond(address(this), _bond);

        _KUMASwap.sellBond(365);
    }

We intend to fix this.

GalloDaSballo commented 1 year ago

The warden has shown a way in which the system could stop selling coupons due to the handling of duplicate coupons, this relies on a specific condition which may not always happen, for this reason, in lack of an attack that can be used to break the functionality, I agree with Medium Severity

c4-judge commented 1 year ago

GalloDaSballo marked the issue as selected for report

code-423n4 / 2023-02-kuma-findings