The current ownership transfer process involves the current owner calling VaderReserve.transferOwnership(). This function checks the new owner is not the zero address and proceeds to write the new owner's address into the owner's state variable. If the nominated EOA account is not a valid account, it is entirely possible the owner may accidentally transfer ownership to an uncontrolled account, breaking all functions with the onlyOwner() modifier.
Consider implementing a two step process where the owner nominates an account and the nominated account needs to call an acceptOwnership() function for the transfer of ownership to fully succeed. This ensures the nominated EOA account is a valid and active account.
Handle
defsec
Vulnerability details
Impact
The current ownership transfer process involves the current owner calling VaderReserve.transferOwnership(). This function checks the new owner is not the zero address and proceeds to write the new owner's address into the owner's state variable. If the nominated EOA account is not a valid account, it is entirely possible the owner may accidentally transfer ownership to an uncontrolled account, breaking all functions with the onlyOwner() modifier.
Proof of Concept
https://github.com/code-423n4/2021-11-vader/blob/607d2b9e253d59c782e921bfc2951184d3f65825/contracts/reserve/VaderReserve.sol#L12
https://github.com/code-423n4/2021-11-vader/blob/607d2b9e253d59c782e921bfc2951184d3f65825/repo/vader-bond/contracts/Treasury.sol#L11
https://github.com/code-423n4/2021-11-vader/blob/607d2b9e253d59c782e921bfc2951184d3f65825/contracts/dex/pool/VaderPoolFactory.sol#L23
Tools Used
Manual code review
Recommended Mitigation Steps
Consider implementing a two step process where the owner nominates an account and the nominated account needs to call an
acceptOwnership()function for the transfer of ownership to fully succeed. This ensures the nominated EOA account is a valid and active account.