Hawk High
First Flight #39
Beginner FriendlySolidity
100 EXP
View results
Previous Next
Submission Details
Impact: low
Likelihood: low
Invalid

Unused Custom Error Code

dynamo

Summary

The contract defines a custom error HH__HawkHighFeesNotPaid() but never actually uses it anywhere in the codebase. This suggests incomplete error handling implementation and could lead to confusion for developers maintaining the code.

Vulnerability Details

// Error is defined but never used
error HH__HawkHighFeesNotPaid();

The enroll function would be the logical place to use this error when fees aren't properly paid, but it relies solely on the SafeERC20 library's built-in error handling instead:

function enroll() external notYetInSession {
// ... validation checks ...
usdc.safeTransferFrom(msg.sender, address(this), schoolFees);
// No custom error handling for failed fee payment
// ... rest of function ...
}

While SafeERC20's safeTransferFrom does revert on failure, utilizing the custom error would provide more descriptive error information specific to this contract's domain logic.

Impact

This issue has low impact because:

  1. The SafeERC20 library appropriately reverts when transfers fail

  2. The contract's functionality is not affected

  3. It represents a minor inconsistency rather than a functional vulnerability

However, it does indicate a lack of attention to detail in error handling implementation and could make the contract harder to maintain or debug.

Tools Used

Recommendations

Either:

  1. Use the defined error in the appropriate context:

// Option 1: Using try/catch with the custom error
try usdc.safeTransferFrom(msg.sender, address(this), schoolFees) returns (bool success) {
if (!success) revert HH__HawkHighFeesNotPaid();
} catch {
revert HH__HawkHighFeesNotPaid();
}
​
// Option 2: Simpler check based on balance changes
uint256 balanceBefore = usdc.balanceOf(address(this));
usdc.safeTransferFrom(msg.sender, address(this), schoolFees);
if (usdc.balanceOf(address(this)) != balanceBefore + schoolFees) {
revert HH__HawkHighFeesNotPaid();
}

Or remove the unused error declaration to avoid confusion:

- error HH__HawkHighFeesNotPaid();

Maintaining consistency between defined errors and their usage improves code readability and makes the contract easier to maintain over time.

Updates

Lead Judging Commences

yeahchibyke Lead Judge 6 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.