Inadequate checks for comptroller in `PoolRegistry#addMarket` allows malicious comptrollers to be added

[code423n4]

Lines of code

https://github.com/code-423n4/2023-05-venus/blob/main/contracts/Pool/PoolRegistry.sol#L255-L327

Vulnerability details

Impact

Malicious comptrollers will be available in the protocol

Proof of Concept

The addMarket function only checks that the input.comptroller is not the 0 address, but does not check if the comptroller was actually created by the PoolRegistry contract. A malicious actor who has been given the "addMarket(AddMarketInput)" role could create a contract that has all the comptroller's functions, but behaves completely differently, and will successfully be able to addMarket with his alien comptroller. The comptroller is one of the core contracts in the protocol, and when there are malicious comptrollers in the protocol, it would severely harm many functionalities in the protocol.

Tools Used

Manual Review

Recommended Mitigation Steps

Consider including this check in addMarket function:

require (_poolByComptroller[comptroller].comptroller==comptroller,"Invalid comptroller address");

Assessed type

Invalid Validation

[c4-judge]

0xean marked the issue as unsatisfactory: Insufficient quality

[Emedudu]

I think this issue is concise and explanatory. Please read the Recommended Mitigation Steps to understand it better.

[0xean]

A malicious actor who has been given the "addMarket(AddMarketInput)" role

Stating this violates security assumptions in the protocol already. This is not a satisfactory report. My decision is final.