code-423n4 / 2023-11-canto-findings

7 stars 6 forks source link

Existing shares can continue operating after blacklisting their bonding curve address #469

Closed c4-submissions closed 10 months ago

c4-submissions commented 10 months ago

Lines of code

https://github.com/code-423n4/2023-11-canto/blob/main/1155tech-contracts/src/Market.sol#L150 https://github.com/code-423n4/2023-11-canto/blob/main/1155tech-contracts/src/Market.sol#L174 https://github.com/code-423n4/2023-11-canto/blob/main/1155tech-contracts/src/Market.sol#L203 https://github.com/code-423n4/2023-11-canto/blob/main/1155tech-contracts/src/Market.sol#L226

Vulnerability details

Impact

Blacklisting a bonding curve does not stop existing shares from operating with it, only ones not yet created. In a scenario where a bonding curve contract was found to not be working as intended and later blacklisted by an admin, the affected shares would still remain in circulation.

Proof of Concept

Go to 1155tech-contracts/src/test/Market.t.sol and place the test case snippet after the setUp function, then run the test suite:

function testBlacklistBondingCurveAndBuy() public {
    market.changeBondingCurveAllowed(address(bondingCurve), true);
    market.restrictShareCreation(false);
    market.createNewShare("My Share", address(bondingCurve), "metadataURI");
    assertEq(market.shareIDs("My Share"), 1);

    market.changeBondingCurveAllowed(address(bondingCurve), false);
    assertFalse(market.whitelistedBondingCurves(address(bondingCurve)));

    token.transfer(address(alice), 1e18);
    vm.startPrank(alice);
    token.approve(address(market), 1e18);
    market.buy(1, 1);
    vm.stopPrank();

    assertEq(token.balanceOf(address(alice)), 1e18 - LINEAR_INCREASE - LINEAR_INCREASE / 10);
}

This will create a share, blacklist the bonding curve and then buy an amount of that share. This will execute successfully and is possible with: buy(), sell(), mintNFT(), and burnNFT().

Tools Used

Manual Analysis

Recommended Mitigation Steps

The simplest solution I can think of is enforcing the following condition in all affected functions:

require(whitelistedBondingCurves[_bondingCurve], "Bonding curve not whitelisted");

However, this introduces a centralization risk that the protocol might not want to have.

Assessed type

Invalid Validation

c4-pre-sort commented 10 months ago

minhquanym marked the issue as duplicate of #473

c4-judge commented 9 months ago

MarioPoneder changed the severity to QA (Quality Assurance)

c4-judge commented 9 months ago

MarioPoneder marked the issue as grade-c