search

hats-finance / illuminex-0x0bb4aa1f58719707405c231fcdf0b405714799cf

0 stars 1 forks source link

`setMinWitness()` is missing access control in `MockTEERollup.sol` #40

Open hats-bug-reporter[bot] opened 4 months ago

hats-bug-reporter[bot] commented 4 months ago

Github username: -- Twitter username: -- Submission hash (on-chain): 0xa765fb82d498f5d6dfd41b8a035377c336b40c7df025f64593e4c3d290457ef7 Severity: high

Description: Description\

Since the contest page mentions all contracts in packages/contracts are in scope so i presume that MockTEERollup.sol is also in scope since its in packages/contracts repo.

setMinWitness() is used to set the minimum witness signatures in MockTEERollup.sol. This function does not have any access control.

    function setMinWitness(uint8 v) public {
        _setMinWitnessSignatures(v);
    }

So when the contract will be deployed on mainnet then setMinWitness() can be called by anyone.

minWitnessSignatures variable will have control by unknown function caller so he can set it to 0 or any high number.

This misuse of setMinWitness() function would break the working of verifyComputations() as it checks the below condition in its function.

    function verifyComputations(FullComputationsProof memory fullProof) public view returns (bool) {

       . . . some code . . . 

        if (fullProof.witnessSignatures.length < minWitnessSignatures) {
            return false;
        }

      . . . some code . . . 
}

All functions using verifyComputations() in BitcoinVerifier would be broken if this function reverts due to this issue.

Recommendations\ Restrict the MockTEERollup.setMinWitness() function with some access control so it can be called by owner/admin address only.

batmanBinary commented 4 months ago

duplicate #20