Core Contracts

Summary

The FeeCollector contract’s timelock update functions, applyTreasuryUpdate and applyRepairFundUpdate, revert with an "UnauthorizedCaller" error when the current time is less than the scheduled effective time. This error message is misleading, as it does not accurately indicate that the timelock delay has not yet elapsed.

Vulnerability Details

In both update functions, the contract checks if the current block timestamp is less than the effective time stored in the corresponding pending update. For example, in applyTreasuryUpdate:

Similarly, applyRepairFundUpdate contains the same check. The issue is that if a caller with the proper authority attempts to apply an update before the timelock delay has passed, the function will revert with an error labeled "UnauthorizedCaller". This error message incorrectly implies that the caller lacks permission, rather than indicating that the update is being attempted prematurely because the timelock period has not yet expired.

Impact

• Developer and Operator Confusion: Misleading error messages can lead operators or developers to incorrectly diagnose the failure as an authorization issue rather than a timing issue.

• Operational Inefficiency: Authorized callers might make unnecessary repeated calls or adjustments in an attempt to bypass the error, unaware that the actual issue is a pending timelock delay.

• Reduced Clarity: The error message does not provide actionable feedback, making troubleshooting and debugging more difficult.

Tools Used

• Manual code review

Recommended Mitigation

1. Define a Specific Error: Create a new error (e.g., error TimelockNotExpired(uint256 currentTime, uint256 effectiveTime);) that more accurately represents the situation.

2. Replace the Error Message: Update the checks in both functions to revert with the new error:

   if (block.timestamp < pendingTreasury.effectiveTime) revert TimelockNotExpired(block.timestamp, pendingTreasury.effectiveTime);