c4-submissions commented 11 months ago

Lines of code

https://github.com/code-423n4/2023-11-canto/blob/335930cd53cf9a137504a57f1215be52c6d67cb3/1155tech-contracts/src/Market.sol#L114-L127

Vulnerability details

Impact

Sharecreators can create new shares even when share creation is restricted.

Proof of Concept

owner() restricts share creation.

function restrictShareCreation(bool _isRestricted) external onlyOwner {
    require(shareCreationRestricted != _isRestricted, "State already set");
    shareCreationRestricted = _isRestricted;
    emit ShareCreationRestricted(_isRestricted);
}

Share creators can call createNewShare() even when share creation is restricted, and it won't revert (Because the onlyShareCreator() modifier has an || operator)

// called by a share creator and passes
function createNewShare(
    string memory _shareName,
    address _bondingCurve,
    string memory _metadataURI
) external onlyShareCreator returns (uint256 id) {
    require(whitelistedBondingCurves[_bondingCurve], "Bonding curve not whitelisted");
    require(shareIDs[_shareName] == 0, "Share already exists");
    id = ++shareCount;
    shareIDs[_shareName] = id;
    shareData[id].bondingCurve = _bondingCurve;
    shareData[id].creator = msg.sender;
    shareData[id].metadataURI = _metadataURI;
    emit ShareCreated(id, _shareName, _bondingCurve, msg.sender);
}

Reason:

// Since this modifier used an || operator, if the caller is a whitelisted share creator, the require statement passes

    modifier onlyShareCreator() {
        require(
            !shareCreationRestricted || whitelistedShareCreators[msg.sender] || msg.sender == owner(),
            "Not allowed"
        );
        _;
    }

The share creator can create as many new shares as he wants when restricted, and the owner() cannot do anything about it.

Tools Used

Manual Review

Recommended Mitigation Steps

modifier onlyShareCreator() {
 if (shareCreationRestricted) {
   require(msg.sender == owner());
    _;
 } else if (!shareCreationRestricted) {
    require(whitelistedShareCreators[msg.sender] || msg.sender == owner());
    _;
 }
_;
}