anon339900 - A malicious user can put any beneficiary address and claim fees from others

[sherlock-admin4]

anon339900

High

A malicious user can put any beneficiary address and claim fees from others

Summary

In the function claim you can put a beneficiary as parameter and collect fees based on the beneficiary but there is no check that msg.sender = beneficiary, which allows theft of funds.

Root Cause

    function claim(address _beneficiary) public nonReentrant {
        // Ensure that the beneficiary has an amount available to claim. We don't revert
        // at this point as it could open an external protocol to DoS.
@>      uint amount = beneficiaryFees[_beneficiary];
        if (amount == 0) return;

        // We cannot make a direct claim if the beneficiary is a pool
        if (beneficiaryIsPool) revert BeneficiaryPoolCannotClaim();

        // Reduce the amount of fees allocated to the `beneficiary` for the token. This
        // helps to prevent reentrancy attacks.
        beneficiaryFees[_beneficiary] = 0;

        // Claim ETH equivalent available to the beneficiary
        IERC20(nativeToken).transfer(_beneficiary, amount);
        emit BeneficiaryFeesClaimed(_beneficiary, amount);
    }

As we can see this is a function that allows beneficiaries to collect fees thorugh this function, but a malicious actor can put any beneficiary that he wants as parameter and claim fees since there is no check to see if _beneficiary == msg.sender. An attacker can monitor the blockchain to see who has a lot of fees building up and claim any of them. There is one instance of this here

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

1. An attacker calls the function claim ,he uses a beneficiary address parameter that has a lot of fees
2. The function does not check for the sender to compare to the beneficiary and the attacker claims funds which are not his

Impact

The attacker can theoretically claim all beneficiary fees.

PoC

No response

Mitigation

Consider using a message sender comparison check e.g.

++ require(msg.sender == _beneficiary, "not beneficiary");