Penalty will be forced even when user wants to lock amount

[code423n4]

Lines of code

https://github.com/code-423n4/2022-05-aura/blob/main/contracts/AuraBalRewardPool.sol#L176

Vulnerability details

Impact

In case shutdown function had been called on AuraLocker, the lock function on AuraLocker will fail. This means Users will be forced to call getReward function with _lock as false which also means that 20% will be deducted from there reward as penalty even though it is not there fault.

Proof of Concept

1. User calls the getReward function with _lock as true
2. The call fails since shutdown is true in AuraLocker which makes the below require fail

require(!isShutdown, "shutdown"); 

3. User can only get reward if _lock is kept as false but this cuts 20% penalty fees even though there is no fault from user side

Similar occurence

** Same issue also holds for claim function at https://github.com/code-423n4/2022-05-aura/blob/main/contracts/AuraMerkleDrop.sol#L114 & AuraVestedEscrow.sol#L61 **

Recommended Mitigation Steps

Ideally getReward function should be revised:

AuraLocker.sol

function getShutDownStatus() external returns(bool) {
        return shutdown;
    }

AuraBalRewardPool.sol#L176

function getReward(bool _lock) public updateReward(msg.sender) returns (bool) {
        uint256 reward = rewards[msg.sender];
        if (reward > 0) {
            rewards[msg.sender] = 0;
			
			// this will prevent the shutdown fees scenario
			if(shutdown){
			rewardToken.safeTransfer(msg.sender, reward);
			}
			
            else if (_lock) {
                auraLocker.lock(msg.sender, reward);
            } else {
                uint256 penalty = (reward * 2) / 10;
                pendingPenalty += penalty;
                rewardToken.safeTransfer(msg.sender, reward - penalty);
            }
            emit RewardPaid(msg.sender, reward, _lock);
        }
        return true;
    }

[0xMaharishi]

Considering this only affects reward tokens, and would only be the case if a significant bug was found in the AuraLocker causing it to shut down.. this should be a 0 or 1 at most.. 3.. LOL

[dmvt]

I agree with sponsor that this is not a high severity issue, but it would cause loss of funds given external factors that are beyond the control of the user. Accordingly, medium makes sense here.

[IllIllI000]

this is the same as #179

[dmvt]

Duplicate of #179