Mylifechangefast_eth - Race condition will occur when submitting block specimen proof

[sherlock-admin]

Mylifechangefast_eth

medium

Race condition will occur when submitting block specimen proof

Summary

n the provided code snippet, there is a potential vulnerability related to the management of the sessionDeadline variable. The vulnerability arises due to the lack of proper synchronization between the check for sessionDeadline being equal to 0 and the subsequent assignment of a new value to sessionDeadline.

Vulnerability Detail

  function submitBlockSpecimenProof(uint64 chainId, uint64 blockHeight, bytes32 blockHash, bytes32 specimenHash, string calldata storageURL) external {
        require(_blockSpecimenProducers.contains(msg.sender), "Sender is not BLOCK_SPECIMEN_PRODUCER_ROLE");
        ChainData storage cd = _chainData[chainId];
        require(cd.nthBlock != 0, "Invalid chain ID");
        require(blockHeight % cd.nthBlock == 0, "Invalid block height");

        BlockSpecimenSession storage session = _sessions[chainId][blockHeight];
        uint64 sessionDeadline = session.sessionDeadline;
        SessionParticipantData storage participantsData = session.participantsData[msg.sender];

        // if this is the first specimen to be submitted for a block, initialize a new session                                                                   |
                                                                                                                                                                                                           |
        if (sessionDeadline == 0) {<---@audit this will cause concurrent and initiate a new session concurrently., see how below   v 
            require(!session.requiresAudit, "Session submissions have closed");

            uint256 currentBlockOnTargetChain = cd.blockOnTargetChain + (((block.number - cd.blockOnCurrentChain) * _secondsPerBlock) / cd.secondsPerBlock);
            uint256 lowerBound = currentBlockOnTargetChain >= cd.allowedThreshold ? currentBlockOnTargetChain - cd.allowedThreshold : 0;
            require(lowerBound <= blockHeight && blockHeight <= currentBlockOnTargetChain + cd.allowedThreshold, "Block height is out of bounds for live sync");

            session.sessionDeadline = uint64(block.number + _blockSpecimenSessionDuration);

            emit SessionStarted(chainId, blockHeight, session.sessionDeadline);

the condition checks if the sessionDeadline is zero, indicating that this is the first specimen submission for a block (as it was initialized to zero before the update).

The require statement ensures that the session is still open for submissions (session.requiresAudit should be false). If requiresAudit is true, it implies that the session has closed, and submissions are no longer accepted.

This line calculates the current block on the target chain by extrapolating based on the elapsed time since the last update of blockOnCurrentChain. It takes into account the time difference, converting it to blocks based on the provided _secondsPerBlock parameter.

It determines the lower bound for the valid block height, considering the allowed threshold. If the current block on the target chain is greater than or equal to the threshold, it subtracts the threshold; otherwise, it sets the lower bound to zero.

This require statement ensures that the submitted block height is within the allowed range for live synchronization. It checks if the block height is greater than or equal to the lower bound and less than or equal to the upper bound.

Scenerio Suppose the smart contract has a session duration of 10 blocks, and a user initiates a session for a specific block height. The user submits a block specimen proof . However, an attacker notices that the contract does not check if the session has concluded. The attacker will keep making the transaction of multiple transactions because of sessionDeadline is 0, indicating no ongoing session in that block.

Initial State:

sessionDeadline is 0, indicating no ongoing session. Concurrent Transactions:

Two transactions simultaneously check sessionDeadline and find it to be 0. Concurrent Session Initialization:

Both transactions calculate currentBlockOnTargetChain and lowerBound. Both transactions set a new value for sessionDeadline, initiating multiple sessions for the same block. Duplicate Session Start:

The result is multiple sessions started concurrently for the same block.

Emitting a just concluded blockspecimenproof of both transactions.

Impact

Code Snippet

   function submitBlockSpecimenProof(uint64 chainId, uint64 blockHeight, bytes32 blockHash, bytes32 specimenHash, string calldata storageURL) external {
        require(_blockSpecimenProducers.contains(msg.sender), "Sender is not BLOCK_SPECIMEN_PRODUCER_ROLE");
        ChainData storage cd = _chainData[chainId];
        require(cd.nthBlock != 0, "Invalid chain ID");
        require(blockHeight % cd.nthBlock == 0, "Invalid block height");

        BlockSpecimenSession storage session = _sessions[chainId][blockHeight];
        uint64 sessionDeadline = session.sessionDeadline;
        SessionParticipantData storage participantsData = session.participantsData[msg.sender];

        // if this is the first specimen to be submitted for a block, initialize a new session
        if (sessionDeadline == 0) {
            require(!session.requiresAudit, "Session submissions have closed");

            uint256 currentBlockOnTargetChain = cd.blockOnTargetChain + (((block.number - cd.blockOnCurrentChain) * _secondsPerBlock) / cd.secondsPerBlock);
            uint256 lowerBound = currentBlockOnTargetChain >= cd.allowedThreshold ? currentBlockOnTargetChain - cd.allowedThreshold : 0;
            require(lowerBound <= blockHeight && blockHeight <= currentBlockOnTargetChain + cd.allowedThreshold, "Block height is out of bounds for live sync");

            session.sessionDeadline = uint64(block.number + _blockSpecimenSessionDuration);

            emit SessionStarted(chainId, blockHeight, session.sessionDeadline);

Tool used

Sleepless night

Recommendation

// Use a locking mechanism to prevent concurrent session initialization bool private sessionInitiated;

function submitBlockSpecimenProof(uint64 chainId, uint64 blockHeight, bytes32 blockHash, bytes32 specimenHash, string calldata storageURL) external {
    // Check if the session is still ongoing
    if(sessionDeadline 0 && block.number <= sessionDeadline, "Session has concluded");

    // Ensure only one transaction can initiate a new session
    require(!sessionInitiated, "Session already initiated");
    sessionInitiated = true;

    // Additional checks...

    // Set a new value for sessionDeadline
    session.sessionDeadline = uint64(block.number + _blockSpecimenSessionDuration);

    // Reset the locking mechanism
    sessionInitiated = false;

    emit SessionStarted(chainId, blockHeight, session.sessionDeadline);
}

Or better still find another way to have a locking mechanism in this function

[sherlock-admin2]

1 comment(s) were left on this issue during the judging contest.

takarez commented:

invalid: watson should provide a POC

[nevillehuang]

Invalid, I don't see the issue of allowing multiple sessions starting on the same block