AuraVestedEscrow's fund can be run with empty amounts

[code423n4]

Lines of code

https://github.com/code-423n4/2022-05-aura/blob/4989a2077546a5394e3650bf3c224669a0f7e690/contracts/AuraVestedEscrow.sol#L91-L110

Vulnerability details

Griefing attack is possible if contract initializing fund() call isn't atomic with constructor. A malicious actor can back-run contract construction with her own version of fund(), initializing the contract with zero amounts. As the initialization is immutable this will yield contract redeployment as AuraVestedEscrow become inoperable as a result of zero amounts initialization.

That's possible as fund has no access controls, while funding amounts aren't checked. The latter is a separate issue as it gives room to operational mistakes of non-malicious actors, having the same contract disabling impact.

Setting severity to medium as system logic breaks up with improper initialization, while the issue is conditional on non-atomic {constructor -> fund} calls execution and fixable with contract redeployment. If the code stays the same and fund() call is non-atomic again, the attack or mistake can be repeated.

Proof of Concept

fund() has no access controls and can be run with zero amounts and totalAmount:

https://github.com/code-423n4/2022-05-aura/blob/4989a2077546a5394e3650bf3c224669a0f7e690/contracts/AuraVestedEscrow.sol#L91-L110

    /**
     * @notice Fund recipients with rewardTokens
     * @param _recipient  Array of recipients to vest rewardTokens for
     * @param _amount     Arrary of amount of rewardTokens to vest
     */
    function fund(address[] calldata _recipient, uint256[] calldata _amount) external nonReentrant {
        require(!initialised, "initialised already");

        uint256 totalAmount = 0;
        for (uint256 i = 0; i < _recipient.length; i++) {
            uint256 amount = _amount[i];

            totalLocked[_recipient[i]] += amount;
            totalAmount += amount;

            emit Funded(_recipient[i], amount);
        }
        rewardToken.safeTransferFrom(msg.sender, address(this), totalAmount);
        initialised = true;
    }

Recommended Mitigation Steps

Consider adding access controls to fund() (or strictly atomic {constructor -> fund} calls execution, but access control is preferred as it completely removes the attack surface in AuraVestedEscrow and all inherited contracts).

In addition to that, consider requiring each amount to be positive.

[dmvt]

This is a gas issue due to the redeployment requirement. The mistake is highly unlikely to occur and has minimal impact if it does.