Skip to content

KUMASwap incorrectly reverts when when _maxCoupons has been reached #10

Open
Open
KUMASwap incorrectly reverts when when _maxCoupons has been reached#10
Labels
2 (Med Risk)Assets not at direct risk, but function/availability of the protocol could be impacted or leak valueM-04bugSomething isn't workingselected for reportThis submission will be included/highlighted in the audit reportsponsor confirmedSponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")

Description

@code423n4
code423n4
opened on Feb 21, 2023

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();
    }

Activity

  1. code423n4
    added
    2 (Med Risk)Assets not at direct risk, but function/availability of the protocol could be impacted or leak value
    bugSomething isn't working
    on Feb 21, 2023
  2. code423n4
    added a commit that references this issue on Feb 21, 2023

    0x52 issue #10

    03b8af5

  3. c4-sponsor commented on Feb 28, 2023

    @c4-sponsor
    c4-sponsor
    on Feb 28, 2023

    m19 marked the issue as sponsor confirmed

  4. c4-sponsor
    added
    sponsor confirmedSponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")
    on Feb 28, 2023

  5. m19 commented on Feb 28, 2023

    @m19
    m19
    on Feb 28, 2023

    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.

  6. GalloDaSballo commented on Mar 1, 2023

    @GalloDaSballo
    GalloDaSballo
    on Mar 1, 2023

    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

  7. c4-judge
    added
    selected for reportThis submission will be included/highlighted in the audit report
    on Mar 1, 2023

  8. c4-judge commented on Mar 1, 2023

    @c4-judge
    c4-judge
    on Mar 1, 2023
    Contributor

    GalloDaSballo marked the issue as selected for report

  9. C4-Staff
    added
    M-04
    on Mar 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    2 (Med Risk)Assets not at direct risk, but function/availability of the protocol could be impacted or leak valueM-04bugSomething isn't workingselected for reportThis submission will be included/highlighted in the audit reportsponsor confirmedSponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions