Method fund() can be called by anyone

[code423n4]

Lines of code

https://github.com/code-423n4/2022-05-aura/blob/main/contracts/AuraVestedEscrow.sol#L96-L110

Vulnerability details

Method fund() can be called by anyone

In the contract AuraVestedEscrow.sol anyone can call the method fund(), and it can only be called once.

Impact

The method fund() can only be called once due to the state variable initialised. There is no check whether the amount is 0 or if the array of recipients is empty.

The method is also the only method setting the values for totalLocked[_recipient], which is used throughout the rest of the contract.

This combined with the method being external and callable by anyone, can create a DoS for that method, and indirectly for the rest of the contract, since all other methods in the contract assumes totalLocked[_recipient] holds some positive value.

Proof of concept

The code can be found at L96-L110:

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;
}

Consider the scenario:

1. Alice is deploying the contract AuraVestedEscrow.sol
2. Bob has waited for the launch of the contract and quickly calls the fund() method with _recipient being an empty array and/or _amount being 0.
3. The variable initialised is now set to true and fund() will from now on revert if called.
4. The following methods will not work anymore or at least not as intended:
   1. cancel() will revert since it requires totalLocked[_recipient] > 0
   2. available() and vested() will always return 0.
   3. claim() will not really do anything as it makes use of avaialble() to calculate the amount to lock/transfer which always returns 0.
5. Thus it succeeded Bob to make the contract more or less unusable.

Recommended mitigation

Do some checking on the input parameters _recipient and _amount. Ideally set some restriction on the method for who can call the method, since it is basically a form of initialization, since it is only callable once.

[0xMaharishi]

You could dos this contract if you were watching the deployment, but there is nothing to gain here for anyone because you also need to transfer the tokens.

With that being said, having it such that the deployer was the only one allowed to fund would solve that issue.

Should be a 1 sev since nothing is at risk

[dmvt]

The contract would need to be redeployed, costing extra gas, so changing this to a Gas issue.

[dmvt]

Grouping this with the warden’s gas report, #131