In function withdrawExpired() of AuraMerkleDrop, don't consider pendingPenalty, and withdraw all the amounts.

[code423n4]

Lines of code

https://github.com/code-423n4/2022-05-aura/blob/4989a2077546a5394e3650bf3c224669a0f7e690/contracts/AuraMerkleDrop.sol#L96-L102

Vulnerability details

Impact

function withdrawExpired() supposed to withdraw expired airdrops that users didn't withdraw them. but it withdraws all the balance of contract and don't consider pendingPenalty which is belong to system and should be transferred to penaltyForwarder.

Proof of Concept

This is withdrawExpired code in AuraMerkleDrop:

    function withdrawExpired() external {
        require(msg.sender == dao, "!auth");
        require(block.timestamp > expiryTime, "!expired");
        uint256 amt = aura.balanceOf(address(this));
        aura.safeTransfer(dao, amt);
        emit ExpiredWithdrawn(amt);
    }

As you can see it transfers all the balance of contract in aura token. but some of that balance could be penalty which most send to penaltyForwarder.

Tools Used

VIM

Recommended Mitigation Steps

first call forwardPenalty() in withdrawExpired() or subtract pendingPenalty from aura.balanceOf(address(this)) when withdrawing.

[0xMaharishi]

Unlikely there would be any pending penalty here but no harm in putting a sub check

[0xMaharishi]

code-423n4/2022-05-aura#6

[dmvt]

If this were to happen, the DAO could simply send the funds to the penalty forwarder. No funds are lost, so this is a QA issue.

[dmvt]

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