Sparkn

CodeFox Inc.
DeFiFoundryProxy
15,000 USDC
View results
Previous Next
Submission Details
Severity: low
Valid

Cannot use block.timestamp or block.timestamp + any value between 0-15 seconds as closeTime

0xanmol

Summary

In the setContest function, if the organizer decides to set closeTime as block.timestamp or block.timestamp+ 15 sec, then the transaction will likely revert which is not the intended behavior protocol want.

Vulnerability Details

The protocol states that if the organizer wishes then he has the ability to immediately end the contest providing the closeTime as block.timestamp when the owner sets the contest.

This is the feature the protocol desires to have because maybe organizers have already decided on the winners and want to just distribute the reward to the winners without any delay.

There are two checks in the setContest function to see if the closeTime is in the past or if it is more than 28 days + currentTime.

if (closeTime > block.timestamp + MAX_CONTEST_PERIOD || closeTime < block.timestamp) {
revert ProxyFactory__CloseTimeNotInRange();
}

Imagine the scenario when the closeTime is block.timestamp + 5s. Note that the closeTime is set off-chain.

Here when we send the transaction with this data then the tx will sit in the mempool and if minner decides to not include this tx in the next block then the whole logic will become invalid and closeTime will be less than the block.timestamp + 5s resulting in the transaction revert.

This is a bit complex issue so I will try to explain it with numbers.I couldn't write a POC for this due to some testing limitations.

  • The etherjs or any frontend library fetch the block.timestamp from the blockchain let's say it is 1000.

  • The closeTime the organizer wants to set is 1000 + 5 = 1005

  • The setContest tx goes to mempool

  • We all know the fact the miner can manipulate the current timestamp between 0-12s.

  • If the miner decided to set the timestamp as 1000(prev time) + 10 s. then our closeTime will be in the past resulting in the transaction revert.

  • In another scenario if the miner sets a block.timetsamp as 1000 + 5s. But he decided to not include our setContest in the block then our closeTime is still in the past so the transaction will revert.

Impact

This will result in the unexpected behavior of the system due to how the blockchain is architectured.

Also, the severity of this depends on how often the organizer sets closeTime like this. If this happens frequently then this can be categorized as high and if not the medium or low. This totally depends of how the organizer wish to set it.

Tools Used

manual review

Recommendations

there are two recommendation.Protocol can use whatever seems best.

  • set some minimum closeTime like 10 mins or soo...

  • Rather than setting the closeTime it is encouraged to set the contest duration.This will break the other part of the system. So use this as only the point of reference

function setContest(address organizer, bytes32 contestId, uint256 contestDuration, address implementation)
public
onlyOwner
{
if (organizer == address(0) || implementation == address(0)) revert ProxyFactory__NoZeroAddress();
if (contestDuration > MAX_CONTEST_PERIOD || contestDuration == 0) revert ProxyFactory__CloseTimeNotInRange();
bytes32 salt = _calculateSalt(organizer, contestId, implementation);
if (saltToCloseTime[salt] != 0) revert ProxyFactory__ContestIsAlreadyRegistered();
saltToCloseTime[salt] = contestDuration + block.timestamp;
emit SetContest(organizer, contestId,contestDuration+block.timestamp, implementation);
}

Support

FAQs

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