It's designed that DAOProxy uses Executor to manage the project and treasury. So DAOProxy can call any external functions in the Executor. But it cannot acceptAdmin(), and it's strange. So, DAOProxy instances (applying LogicV2) will not be able to acceptAdmin(). So, new admins will be EOAs or some new smart contract, but never DAOProxys with LogicV2.
This limitation looks strange.
Proof of Concept
NounsDAOExecutor.setPendingAdmin()
then no DAOProxy instance can acceptAdmin()
Tools Used
VisualCode
Recommended Mitigation Steps
Add function in the NounsDAOLogicV2.sol to acceptAdmin() in the NounsDAOExecutor.
Lines of code
https://github.com/code-423n4/2022-08-nounsdao/blob/45411325ec14c6d747b999a40367d3c5109b5a89/contracts/governance/NounsDAOExecutor.sol#L89-L95
Vulnerability details
Impact
It's designed that DAOProxy uses Executor to manage the project and treasury. So DAOProxy can call any external functions in the Executor. But it cannot acceptAdmin(), and it's strange. So, DAOProxy instances (applying LogicV2) will not be able to acceptAdmin(). So, new admins will be EOAs or some new smart contract, but never DAOProxys with LogicV2. This limitation looks strange.
Proof of Concept
NounsDAOExecutor.setPendingAdmin() then no DAOProxy instance can acceptAdmin()
Tools Used
VisualCode
Recommended Mitigation Steps
Add function in the NounsDAOLogicV2.sol to acceptAdmin() in the NounsDAOExecutor.