Closed code423n4 closed 2 years ago
This function exists for the purpose of initiating GovernorBravoDelegate before any proposals have been queried. After the first proposal is queried initial proposalID is non-zero and the Timelock Address will be fixed.
Agree with the sponsor, also not only will the function be callable with non-zero arguments only once, the timelock will also revert on second call per: https://github.com/Plex-Engineer/lending-market-v2/blob/443a8c0fed3c5018e95f3881a31b81a555c42b2d/contracts/Timelock.sol#L47-L48
require(msg.sender == pendingAdmin, "Timelock::acceptAdmin: Call must come from pendingAdmin.");
Lines of code
https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/Governance/GovernorBravoDelegate.sol#L127-L131
Vulnerability details
Submitting as med risk because documentation inside functions shows that require is there for the purpose of preventing it being called again but is not working correctly, and since we do not have access to the timelock code I cannot fully assess the impact
Impact
_initiate() can be called multiple times
Proof of Concept
L129 uses a require to check that initialProposalID == 0 but initialProposalID is never set anywhere in either this function or anywhere else meaning that this require will never revert, allows the function to be called multiple times
Tools Used
Recommended Mitigation Steps
Set inititalProposalID to something other than 0 in the _initiate function