New Admin is not set correctly

[code423n4]

Lines of code

https://github.com/code-423n4/2022-05-aura/blob/main/contracts/AuraStakingProxy.sol#L116
https://github.com/code-423n4/2022-05-aura/blob/main/contracts/VoterProxy.sol#L73
https://github.com/code-423n4/2022-05-aura/blob/main/contracts/AuraVestedEscrow.sol#L77

Vulnerability details

We will take applyPendingOwner function from AuraStakingProxy.sol#L116 in this POC

Impact

The require statement condition in applyPendingOwner function is incorrect. msg.sender should be pendingOwner and not owner. Owner might give an incorrect pendingOwner address, but a 2 step process where pendingOwner has to accept the Admin will confirm that address was correct

Proof of Concept

1. Admin sets a new pendingOwner, by mistake he sets incorrect address for pendingOwner
2. Unaware of the mistake, Admin calls applyPendingOwner which sets admin to this incorrect pendingOwner
3. Now admin is controlled by an incorrect address and all admin functions cant be used

Similar occurrence

1. This also need to be fixed in setOwner function at VoterProxy.sol#L73
2. setAdmin function at AuraVestedEscrow.sol#L77 - It should be checked whether the new Admin is not an incorrect address(0). Also it would be better to have a 2 step approach for updating admin as if set wrong then all admin operations will halt

require(_admin!=address(0),"Incorrect address");

Recommended Mitigation Steps

function applyPendingOwner() external {
        //require(msg.sender == owner, "!auth"); // Incorrect
        require(msg.sender == pendingOwner, "!auth");
		require(pendingOwner != address(0), "invalid owner");

        owner = pendingOwner;
        pendingOwner = address(0);
    }

[0xMaharishi]

This is definitely not a 2 risk.. 0 or 1 considering there is nothing particularly damaging happening here and it's an optimisation issue

[dmvt]

Per #364 (comment) I have decided to downgrade this to QA.

[dmvt]

Grouping this with the warden’s QA report, #47