sherlock-audit / 2023-11-covalent-judging

3 stars 2 forks source link

themis210 - without checking _minSubmissionRequired, there can be Division error in BlockSpecimenProofChain::finalizeSpecimenSession #36

Closed sherlock-admin closed 7 months ago

sherlock-admin commented 7 months ago

themis210

medium

without checking _minSubmissionRequired, there can be Division error in BlockSpecimenProofChain::finalizeSpecimenSession

Summary

In BlockSpecimenProofChain::finalizeSpecimenSession, when the number of submissions is sufficient and the quorum is achieved, there can be zero division error

https://github.com/sherlock-audit/2023-11-covalent/blob/main/cqt-staking/contracts/BlockSpecimenProofChain.sol#L431 Here, when accidently the _minSubmissionRequired is set to 0, it can possible make 0 division.

Vulnerability Detail

In rare case, when accidently the _minSubmissionRequired is set to 0, then there's no way to prevent 0 division in finalizeSpecimenSession , It can halt the finalizing process and cause big problem.

Impact

It can halt the finalizing specimen session process and cause loss of session.

Code Snippet

        for (uint256 i = 0; i < blockSpecimenHashesLength; i++) {
            rawBlockHash = blockHashesRaw[i];
            BlockProperties storage bh = session.blockProperties[rawBlockHash];
            for (uint256 j = 0; j < bh.specimenHashes.length; j++) {
                specimenHash = bh.specimenHashes[j];
                uint256 len = bh.participants[specimenHash].length;
                contributorsN += len;
                if (len > max) {
                    max = len;
                    agreedBlockHash = rawBlockHash;
                    agreedSpecimenHash = specimenHash;
                }
            }
        }
        // check if the number of submissions is sufficient and if the quorum is achieved
        if (_minSubmissionsRequired <= max && (max * _DIVIDER) / contributorsN > _blockSpecimenQuorum) {
            // TODO: doesn't free session space. Though it should.
            _finalizeWithParticipants(session, chainId, blockHeight, agreedBlockHash, agreedSpecimenHash);
        } else emit QuorumNotReached(chainId, blockHeight);

In here, need to make sure the _minSubmissionsRequired is greater than 0

Tool used

Manual Review

Recommendation

In finalizeSpecimenSession, please check the _minSubmissionsRequired is greater than 0

sherlock-admin2 commented 7 months ago

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

takarez commented:

invalid: only admin will set the value

noslav commented 7 months ago

resolved by set lower and upper bounds for minimum submission of proofs - sa30

nevillehuang commented 7 months ago

Invalid, similar reasonings to #30