sherlock-audit / 2023-12-avail-judging

4 stars 4 forks source link

soliditywala - Fees can be burned due to allowing feeRecipient to be address 0 #70

Closed sherlock-admin2 closed 8 months ago

sherlock-admin2 commented 8 months ago

soliditywala

medium

Fees can be burned due to allowing feeRecipient to be address 0

Summary

The withdrawFees() function in the AvailBridge smart contract, callable by anyone, poses a vulnerability that allows malicious actors to intentionally burn accumulated fees. This risk arises when the feeRecipient is set to address(0) using the updateFeeRecipient() function.

Vulnerability Detail

The withdrawFees() function lacks a proper check on the feeRecipient address, enabling any user, including malicious actors, to deliberately call the function when feeRecipient is set to address(0). This results in the burning of fees, as they are sent to an unrecoverable address.

Impact

Malicious actors can intentionally trigger withdrawFees() when feeRecipient is set to address(0), leading to the deliberate burning of accumulated fees.

Code Snippet

    function withdrawFees() external {
        uint256 fee = fees;
        delete fees;
        // @audit fees can be burned if called when **feeRecipient** is not set
        // slither-disable-next-line low-level-calls
        (bool success, ) = feeRecipient.call{value: fee}("");
        if (!success) {
            revert WithdrawFailed();
        }
    }

Tool used

Manual Review

Recommendation

To mitigate this vulnerability, implement a check in the withdrawFees() function to ensure that only a valid (non-zero) feeRecipient can receive fee. Additionally, enhance the updateFeeRecipient() function to disallow setting address(0) as the fee recipient, preventing intentional fee burning by malicious actors.

function withdrawFees() external {
    require(feeRecipient != address(0), "Invalid feeRecipient");
    uint256 fee = fees;
    delete fees;
    (bool success, ) = feeRecipient.call{value: fee}("");
    if (!success) {
        revert WithdrawFailed();
    }
}

function updateFeeRecipient(address newFeeRecipient) external onlyRole(DEFAULT_ADMIN_ROLE) {
    require(newFeeRecipient != address(0), "Invalid feeRecipient");
    feeRecipient = newFeeRecipient;
}
sherlock-admin commented 8 months ago

2 comment(s) were left on this issue during the judging contest.

tsvetanovv commented:

Invalid

takarez commented:

invalid because { invalid: governance role}