BriVault
First Flight #52
Beginner FriendlySolidity
100 EXP
View results
Previous Next
Submission Details
Impact: high
Likelihood: medium
Invalid

The unverified setting of the constructor parameter led to the failure of the DDOS contract.

eigaku6326

Root + Impact

Description

  • In the contract constructor, the start and end times of the activity should be correctly initialized to ensure that the end time of the activity comes after the start time, so as to guarantee a normal activity process.

  • In the current implementation, the constructor does not verify the relationship between eventStartDate and eventEndDate, allowing the end time to be set before the start time. This can lead to logical errors and potential denial of service.

constructor (IERC20 _asset, uint256 _participationFeeBsp, uint256 _eventStartDate, address _participationFeeAddress, uint256 _minimumAmount, uint256 _eventEndDate) ERC4626 (_asset) ERC20("BriTechLabs", "BTT") Ownable(msg.sender) {
@> if (_participationFeeBsp > PARTICIPATIONFEEBSPMAX){
@> revert limiteExceede();
@> }
@> // Missing validation for eventStartDate and eventEndDate relationships
participationFeeBsp = _participationFeeBsp;
eventStartDate = _eventStartDate;
eventEndDate = _eventEndDate;
participationFeeAddress = _participationFeeAddress;
minimumAmount = _minimumAmount;
_setWinner = false;
}

Risk

Likelihood:

  • Reason 1 This situation occurs when the contract deployer accidentally or intentionally sets incorrect time parameters.

  • Reason 2 Since the constructor is a one-time operation, once a deployment error occurs, it cannot be modified, and the risks have a long-lasting impact.

Impact:

  • Impact 1 Logical error - When eventEndDate is earlier than eventStartDate, the time check in the setWinner function will exhibit abnormal behavior.

  • Impact 2 Potential Denial of Service - Although technically, the setWinner function can execute normally after the actual activity starts (when block.timestamp > eventStartDate, it is also guaranteed that block.timestamp > eventEndDate will be true), this design still tends to cause confusion and potential logical issues.

Proof of Concept

// Example of constructor parameters:
// _eventStartDate = 100 (timestamp for a future date)
// _eventEndDate = 50 (timestamp earlier than the start date)
// After deployment, what will happen when the setWinner() function is called:
// - If the current time is less than 50: an eventNotEnded error will be triggered
// - If 50 < current time < 100: the setWinner() function can be successfully called, although the activity is "technically" not yet started
// - If the current time is greater than 100: the setWinner() function can be successfully called
This behavior goes against common sense because it is usually the case that the winner should be determined after the activity has ended (after the start time and after the end time).

Recommended Mitigation

constructor (IERC20 _asset, uint256 _participationFeeBsp, uint256 _eventStartDate, address _participationFeeAddress, uint256 _minimumAmount, uint256 _eventEndDate) ERC4626 (_asset) ERC20("BriTechLabs", "BTT") Ownable(msg.sender) {
​
if (_participationFeeBsp > PARTICIPATIONFEEBSPMAX){
revert limiteExceede();
}
+ if (_eventEndDate <= _eventStartDate) {
+ revert InvalidEventDates();
+ }
+ // Make sure that the end time of the activity is after the start time.
participationFeeBsp = _participationFeeBsp;
eventStartDate = _eventStartDate;
eventEndDate = _eventEndDate;
participationFeeAddress = _participationFeeAddress;
minimumAmount = _minimumAmount;
_setWinner = false;
}
​
+ error InvalidEventDates();
Updates

Appeal created

bube Lead Judge 19 days ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Missing Constructor Validation

This is owner action and the owner is assumed to be trusted and to provide correct input arguments.

Support

FAQs

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

Give us feedback!