search

code-423n4 / 2023-10-wildcat-findings

12 stars 9 forks source link

If the ArchController removes a market controller or a market it can't be added back #662

Closed c4-submissions closed 10 months ago

c4-submissions commented 10 months ago

Lines of code

https://github.com/code-423n4/2023-10-wildcat/blob/main/src/WildcatArchController.sol#L149-L161 https://github.com/code-423n4/2023-10-wildcat/blob/main/src/WildcatArchController.sol#L192-L204 https://github.com/code-423n4/2023-10-wildcat/blob/main/src/WildcatMarketControllerFactory.sol#L294-L299 https://github.com/code-423n4/2023-10-wildcat/blob/main/src/WildcatMarketController.sol#L351-L357

Vulnerability details

Impact

Once ArchController removes a controller or market from the list, it can't be added back. As a result, removing controller is akin to removing a borrower, because he won't be able to create a new one, since the controller address is derived from the borrower's address.

https://github.com/code-423n4/2023-10-wildcat/blob/main/src/WildcatMarketControllerFactory.sol#L288

As for the market, it won't be able to deploy a sentinel escrow, thus sanctioned users will stay unblocked

https://github.com/code-423n4/2023-10-wildcat/blob/main/src/WildcatSanctionsSentinel.sol#L95-L102 https://github.com/code-423n4/2023-10-wildcat/blob/main/src/market/WildcatMarketBase.sol#L172

Proof of Concept

From the ArchController code we can see that only a factory can register a controller, and only a controller can a register a market. This can only be done once because we add these contracts after deployment and cannot deploy twice at the same address

https://github.com/code-423n4/2023-10-wildcat/blob/main/src/WildcatMarketControllerFactory.sol#L294-L299 https://github.com/code-423n4/2023-10-wildcat/blob/main/src/WildcatMarketController.sol#L351-L357

Tools Used

Manual review

Recommended Mitigation Steps

Allow the owner of the ArchController to add controllers and markets to the list in case they were removed by mistake or need to be re-added.

Assessed type

Other

c4-pre-sort commented 10 months ago

minhquanym marked the issue as duplicate of #31

c4-judge commented 10 months ago

MarioPoneder changed the severity to QA (Quality Assurance)

c4-judge commented 10 months ago

MarioPoneder marked the issue as grade-c

k4zanmalay commented 10 months ago

I believe this issue can be a duplicate of https://github.com/code-423n4/2023-10-wildcat-findings/issues/498 , see

As for the market, it won't be able to deploy a sentinel escrow, thus sanctioned users will stay unblocked

https://github.com/code-423n4/2023-10-wildcat/blob/main/src/WildcatSanctionsSentinel.sol#L95-L102 https://github.com/code-423n4/2023-10-wildcat/blob/main/src/market/WildcatMarketBase.sol#L172

MarioPoneder commented 10 months ago

Thank you for your comment!

The present issue does not identify the withdrawal DoS core impact of #498.
Furthermore, it is lacking quality (level of detail) as well as proof.

For those reasons it cannot be counted as a duplicate and the current QA grade seems appropriate.