Re-Entrant Bond Purchase Flow

[code423n4]

Lines of code

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

Vulnerability details

Impact

The code of KUMASwap::buyBond is re-entrant; whenever the face value of a bond is is greater than the realized bond value an EIP-721 clone is created via the KBCToken::issueBond function which in turn invokes the _safeMint implementation of OpenZeppelin's ERC721Upgradeable.

As the _safeMint function will invoke the onERC721Received function on the recipient, the recipient is able to re-enter the contact mid-execution. After analyzing the contract's codebase, one can identify the following misbehaviors:

• The _minCoupon value will possess an incorrect value during the re-entrancy
• The KBCToken can be utilized before its entries are initialized in the KUMASwap contract and before the KIBToken amount has been burned

The claimBond which relies on the _cloneBonds data entry is safe, however, as the data entry is not written yet during the re-entrant call. Additionally, the _minCoupon value while adequately maintained by the contract is not actively utilized as a sanitization measure in the contract which is slightly ambiguous as it should be utilized to either enforce a purchase of a bond to be of the _minCoupon or a sale of a bond to be lower than the _minCoupon, or some other form of business logic.

Proof of Concept

The following smart contract will cause the ReentryAchieved event to be emitted if it has sufficient KIB tokens for the VALID_TOKEN_ID's purchase:

contract KumaReentrancy {
    IKUMASwap public swap = IKUMASwap(ADDRESS_OF_SWAP_HERE);
    uint256 constant public VALID_TOKEN_ID = X;

    event ReentryAchieved();

    function reenter() external {
        swap.buyBond(VALID_TOKEN_ID);
    }

    function onERC721Received(address operator, address from, uint256 tokenId, bytes calldata data)
        external
        returns (bytes4)
    {
        emit ReentryAchieved();
        return IERC721Receiver.onERC721Received.selector;
    }
}

Tools Used

Manual review.

Recommended Mitigation Steps

We advise the clone bond to be assigned in an else clause introduced to the final if conditional of the code. This will ensure that the code of KUMASwap::buyBond conforms to the Checks-Effects-Interactions pattern and thus contains a correct system state during the external call permitting re-entrancies.

[c4-judge]

GalloDaSballo marked the issue as primary issue

[GalloDaSballo]

Primary because simplified POC

[GalloDaSballo]

Am not convinced that the finding is Medium Severity because of a specific leak of value, the finding is valid but am thinking QA low

[c4-sponsor]

m19 marked the issue as disagree with severity

[m19]

We also disagree with severity, we agree there's a re-entracy but there is no way to exploit it that we can see.

[GalloDaSballo]

Downgrading to QA in lack of a leak of value

I recommend the warden to follow up with the Sponsor if they believe there's cause for further concern, however, due to lack of proof, am downgrading to QA

[GalloDaSballo]

L

[c4-judge]

GalloDaSballo changed the severity to QA (Quality Assurance)

[c4-judge]

GalloDaSballo marked the issue as grade-a

[c4-judge]

GalloDaSballo marked the issue as selected for report

[c4-judge]

GalloDaSballo marked the issue as not selected for report