KiteWeb3 - Potential Denial-of-Service (DoS) Risk Due to Lack of Upper Limit and Absence of Event Emission in ```AvailBridge::updateFeePerByte()```

[sherlock-admin]

KiteWeb3

medium

Potential Denial-of-Service (DoS) Risk Due to Lack of Upper Limit and Absence of Event Emission in AvailBridge::updateFeePerByte()

Summary

The AvailBridge::updateFeePerByte() function is responsible for updating the feePerByte charged for message transfers across the bridge. This function is callable only by addresses with the DEFAULT_ADMIN_ROLE, which is intended to be a multisig address.

Vulnerability Detail

• Lack of Upper Limit: the function does not enforce an upper limit on the feePerByte. This could potentially allow the multisig to set the fee to an exorbitantly high value, effectively preventing users from using the bridge's services and causing a DoS condition.
• Absence of Event Emission: the function does not emit an event upon updating the fee. This lack of transparency means that users and external observers are not notified of changes to the fee structure, which could lead to unexpected costs and hinder auditability and governance oversight.

Note: The multisig management rules aren't yet defined so it doesn't ensure the necessary security level. For example, the signers could be set to 1/3. This means that 1 signer can set the new fees and change it.

Impact

• Operational Disruption: a prohibitively high fee could prevent users from using the bridge, effectively causing a DoS attack on the service.
• Lack of Transparency: without event logs, users and external observers cannot easily track changes to the fee structure, leading to a potential loss of trust and difficulties in contract monitoring.
• The use of a multisig as a trusted role can't guarantee in toto the prevention of the attack: --- Compromised Multisig: if the multisig address is compromised due to poor security practices an attacker could unilaterally set the fee to an exorbitantly high value, effectively halting the bridge's operations. --- Malicious Intent: if one or more parties controlling the multisig act with malicious intent, they could intentionally set the fee so high that it would prevent users from using the bridge, effectively causing a DoS condition. --- Accidental Misuse: even without malicious intent, the multisig signatories could mistakenly set the fee too high, resulting in temporary unintended disruption of the service.

Code Snippet

Link to the code snippet: https://github.com/sherlock-audit/2023-12-avail/blob/main/contracts/src/AvailBridge.sol#L153C5-L155C6

function updateFeePerByte(uint256 newFeePerByte) external onlyRole(DEFAULT_ADMIN_ROLE) {
@>        feePerByte = newFeePerByte;
    }

Tool used

Manual Review

Recommendation

• Implement Fee Cap: introduce a reasonable upper limit for the feePerByteto prevent setting it to a disruptive value.
• Emit Update Event: modify the updateFeePerByte() function to emit an event that logs the old fee, the new fee, and the admin address responsible for the change.
• Enhance Multisig Controls: define clear and transparent rules for the multisig signers.
• Introduce Timelock: add a time delay for fee updates to provide users with advance notice and the ability to react to changes.

// Event to be emitted when the fee per byte is updated
+ event FeePerByteUpdated(uint256 oldFeePerByte, uint256 newFeePerByte, address indexed admin);

function updateFeePerByte(uint256 newFeePerByte) external onlyRole(DEFAULT_ADMIN_ROLE) {
+    require(newFeePerByte <= MAX_FEE_PER_BYTE, "Fee exceeds upper limit");
    uint256 oldFee = feePerByte;
    feePerByte = newFeePerByte;
+    emit FeePerByteUpdated(oldFee, newFeePerByte, msg.sender);
}

[sherlock-admin]

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

takarez commented:

invalid because { invalid: low or even suggestion}