In the current design/implementation, CollateralToken is minted and burned by COLLATERAL_MINTER_ROLE and COLLATERAL_BURNER_ROLE.
Such roles can be granted by the owner of QuantConfig, which is the deployer of the
QuantConfig.
Once the attacker managed to acquire on of these roles, they will be able to rug users.
PoC
A compromised/malicious minter of CollateralToken.sol can mint an arbitrary amount and redeem before other users.
If there is a Uni v2 liquidity pool for that CollateralToken, a compromised/malicious burner of CollateralToken.sol can burn all the tokens from that liquidity pool and rug all the liquidity providers.
Recommendation
CollateralToken.sol should not require a special role to mint or burn.
mintCollateralToken() should take in the funds required to mint and mint to the caller permissionlessly;
burnCollateralToken() should burn the desired amount from the msg.sender and send the redeemed funds to the caller, also permissionlessly.
This way, we can eliminate the centralization risk caused by privileged roles in CollateralToken.sol.
With the changes above, the non-upgradable CollateralToken.sol will be holding the underlying tokens instead of the upgradeable Controller.sol. Which will further reduce the centerlization risk.
Lines of code
https://github.com/code-423n4/2022-03-rolla/blob/a06418c9cc847395f3699bdf684a9ac066651ed7/quant-protocol/contracts/options/CollateralToken.sol#L100-L117
Vulnerability details
In the current design/implementation,
CollateralToken
is minted and burned byCOLLATERAL_MINTER_ROLE
andCOLLATERAL_BURNER_ROLE
.Such roles can be granted by the owner of
QuantConfig
, which is the deployer of theQuantConfig
.Once the attacker managed to acquire on of these roles, they will be able to rug users.
PoC
A compromised/malicious minter of
CollateralToken.sol
can mint an arbitrary amount and redeem before other users.If there is a Uni v2 liquidity pool for that CollateralToken, a compromised/malicious burner of
CollateralToken.sol
can burn all the tokens from that liquidity pool and rug all the liquidity providers.Recommendation
CollateralToken.sol
should not require a special role to mint or burn.mintCollateralToken()
should take in the funds required to mint and mint to the caller permissionlessly;burnCollateralToken()
should burn the desired amount from themsg.sender
and send the redeemed funds to the caller, also permissionlessly.This way, we can eliminate the centralization risk caused by privileged roles in
CollateralToken.sol
.With the changes above, the non-upgradable
CollateralToken.sol
will be holding the underlying tokens instead of the upgradeableController.sol
. Which will further reduce the centerlization risk.