code-423n4 / 2023-10-nextgen-findings

5 stars 3 forks source link

Missing deadline checks #2032

Closed c4-submissions closed 11 months ago

c4-submissions commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/main/smart-contracts/MinterContract.sol#L461

Vulnerability details

Consider addings implementation to handle the expiration of the transaction for additional security.

To implement a transaction expiration mechanism in the emergencyWithdraw add a timestamp check to ensure that the transaction is executed only within a certain timeframe. This can be achieved by introducing a state variable to store the deadline and then checking against this deadline within the function.

First, you would introduce a state variable and a function to set the deadline. This could be set when the emergency situation is declared or when the contract is initialized.

uint256 public withdrawalDeadline;

function setWithdrawalDeadline(uint256 _deadline) public onlyOwner {
    withdrawalDeadline = _deadline;
}

Here, onlyOwner should be a modifier that allows only the contract owner to set the deadline. The _deadline would typically be a timestamp in Unix time.

Then, modify the emergencyWithdraw function to check against this deadline:

function emergencyWithdraw() public FunctionAdminRequired(this.emergencyWithdraw.selector) noReentrant {
    require(block.timestamp <= withdrawalDeadline, "Transaction expired");

    uint balance = address(this).balance;
    address admin = adminsContract.owner();
    require(admin != address(0), "Admin address is zero");

    uint256 gasLimit = someGasLimit; // Define according to your contract's requirements

    (bool success, ) = payable(admin).call{value: balance, gas: gasLimit}("");
    emit Withdraw(msg.sender, success, balance);
}

In this setup, the emergencyWithdraw function will fail if the current block timestamp is later than the withdrawalDeadline. This prevents the function from being called after the specified deadline, adding an extra layer of security and control over the emergency withdrawal process.

Remember, when setting the deadline, ensure the timestamp is in the future and reasonable based on the expected timeframe for the emergency situation. Also, be aware of the time zone considerations when working with timestamps in Solidity, as they are in UTC.

Assessed type

Invalid Validation

c4-pre-sort commented 1 year ago

141345 marked the issue as sufficient quality report

c4-sponsor commented 1 year ago

a2rocket (sponsor) disputed

a2rocket commented 1 year ago

this is not the case here, emergency means emergency so we need to be able to call it whenever its deemed very very urgent.

alex-ppg commented 11 months ago

The Warden advises the Sponsor to introduce time-based restrictions on the emergency functionality of the protocol.

Centralization issues are generally OOS and in this particular scenario as the Sponsor has specified it might arguably be better to not enforce a time-based threshold.

The Sponsor is of course advised to implement some form of governance mechanism or multi-signature wallet to be in charge of these centralized functions instead of a single EOA to minimize their potential off-chain attack susceptibility.

c4-judge commented 11 months ago

alex-ppg marked the issue as unsatisfactory: Out of scope