code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-03-rolla/blob/main/quant-protocol/contracts/options/CollateralToken.sol#L101-L117

Vulnerability details

Impact

    function mintCollateralToken(
        address recipient,
        uint256 collateralTokenId,
        uint256 amount
    ) external override {
        require(
            quantConfig.hasRole(
                quantConfig.quantRoles("COLLATERAL_MINTER_ROLE"),
                msg.sender
            ),
            "CollateralToken: Only a collateral minter can mint CollateralTokens"
        );

        emit CollateralTokenMinted(recipient, collateralTokenId, amount);

        _mint(recipient, collateralTokenId, amount, "");
    }

Using the mintCollateralToken() function of CollateralToken, an address with COLLATERAL_MINTER_ROLE can mint an arbitrary amount of tokens.

If the private key of the deployer or an address with the COLLATERAL_MINTER_ROLE is compromised, the attacker will be able to mint an unlimited amount of collateral tokens.

We believe this is unnecessary and poses a serious centralization risk.

Proof of Concept

https://github.com/code-423n4/2022-03-rolla/blob/main/quant-protocol/contracts/options/CollateralToken.sol#L101-L117 https://github.com/code-423n4/2022-03-rolla/blob/main/quant-protocol/contracts/options/CollateralToken.sol#L138-L160

Tools Used

None

Recommended Mitigation Steps

Consider removing the COLLATERAL_MINTER_ROLE, make the CollateralToken only mintable by the owner, and make the Controller contract to be the owner and therefore the only minter.

alcueca commented 2 years ago

From a comment in #47:

"The roles are renounced as per our deployment config covered in the docs. But this bug is still valid as the role OPTIONS_MINTER_ROLE can be reassigned".

Taking this one as main, with the vulnerability being that several the MINTER and BURNER roles can be reassigned and have unnecessary powers that can be used to rug users.

0xca11 commented 2 years ago

All roles were removed from the protocol, and now only the Controller contract can mint QTokens and CollateralTokens.

https://github.com/RollaProject/quant-protocol/pull/90

code-423n4 / 2022-03-rolla-findings