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 10 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.

Give us feedback!