hats-finance / illuminex-0x0bb4aa1f58719707405c231fcdf0b405714799cf

0 stars 1 forks source link

Missing events for `verifyComputations`, `processDeposit` (low) #25

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

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

Github username: -- Twitter username: GitTopStar Submission hash (on-chain): 0xebd2e6520c3c1dd8fdbdb6f22d7e912770c767f1ac415062e846060c3c3fd247 Severity: low

Description: Missing events for verifyComputations, processDeposit (low)

Description\

The verifyComputations function in the TEERollup contract currently lacks event emission. This omission reduces transparency and traceability of significant actions, which are crucial for security, auditing, and debugging purposes.

The verifyComputations function is a critical part of the TEERollup contract as it verifies computations results using contract and witness signatures. However, it does not emit any events upon successful or failed verification attempts. Events are essential in smart contracts for logging important activities, which are recorded on the blockchain and can be accessed by off-chain applications. However, none of the above functions emits events.

Attachments

  1. Proof of Concept (PoC) File
    
    TEERollup.sol

function verifyComputations(FullComputationsProof memory fullProof) public view returns (bool) { bool isContractSignatureValid = Sapphire.verify( Sapphire.SigningAlg.Secp256k1PrehashedKeccak256, _keyPair.publicKey, abi.encodePacked(keccak256(fullProof.partialProof.computationsResult)), "", fullProof.partialProof.contractSignature );

    if (!isContractSignatureValid) {
        return false;
    }

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

    bytes[] memory _usedPubKeys = new bytes[](fullProof.witnessSignatures.length);

    for (uint i = 0; i < fullProof.witnessSignatures.length; i++) {
        if (!witnessPublicKeysSet[fullProof.witnessSignatures[i].publicKey]) {
            return false;
        }

        for (uint j = 0; j < _usedPubKeys.length; j++) {
            if (keccak256(_usedPubKeys[j]) == keccak256(fullProof.witnessSignatures[i].publicKey)) {
                return false;
            }
        }

        _usedPubKeys[i] = fullProof.witnessSignatures[i].publicKey;

        bool isWitnessSignatureValid = Sapphire.verify(
            Sapphire.SigningAlg.Secp256k1PrehashedKeccak256,
            fullProof.witnessSignatures[i].publicKey,
            abi.encodePacked(keccak256(fullProof.partialProof.computationsResult)),
            "",
            fullProof.witnessSignatures[i].signature
        );

        if (!isWitnessSignatureValid) {
            return false;
        }
    }

@> //emit event is missing return true; }

BitcoinAbstractWallet.sol
function processDeposit(
    Transaction memory _tx,
    bytes memory _data
) public override {
    require(msg.sender == prover);
    _processDeposit(_tx, _data);

@> //emit event is missing }



2. **Revised Code File (Optional)**

**Recommendation to fix**
Emit events in `verifyComputations()`, `processDeposit()` functions.
Note Issue is applicable to all such instances where events are missing.
0xEricTee commented 4 months ago

This issue is invalid as verifyComputations is a view function and processDeposit function did emit event inside internal function _processDeposit.

rotcivegaf commented 4 months ago

Informational