Contract fails to deliver promised returns, but doesn't lose value
Description
Summary
pendingAdmin will not be able to call acceptAdminOwnership due to onlyAdmin modifier.
Vulnerability Details
AlchemistCurator function allows transferring of contract admin ownership through function transferAdminOwnerShip, the process is however a two step process. In the first step the admin call function with _newAdmin to set as pendingAdmin.
function transferAdminOwnerShip(address _newAdmin) external onlyAdmin {
pendingAdmin = _newAdmin;
}
The second step involves accepting of ownership by the pendingAdmin which however restrict the call by pendingAdmin as it is callable only by admin due to onlyAdmin modifier.
This enforces the authority to present admin to call both function and does not allow the pendingAdmin to call the acceptAdminOwnership function.
The recommendation is made to remove the onlyAdmin modifier and implementing check that allows pendingAdmin to call the acceptAdminOwnership function.
Proof of Concept
Proof of Concept
Here is a test demonstrating the issue of restriction made making pendingAdmin to call the acceptAdminOwnership function.
First we call the function transferAdminOwnerShip with admin making the call the transaction process normally, but when we try to call the acceptAdminOwnershipfunction with thependingAdmin``.
The result clearly shows the testAcceptAdminOwnership revert with reason PD showing pendingAdmin unable to call the function acceptAdminOwnership of contract AlchemistCurator.
