hats-finance / illuminex-0x0bb4aa1f58719707405c231fcdf0b405714799cf

0 stars 0 forks source link

`compute()` wont work correctly #38

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

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

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

Description: Description\

compute() in TEERollup.sol is implemented as:

    function compute(bytes calldata input) public view returns (PartialComputationsProof memory) {
        bytes memory result = _compute(input);
        bytes memory signature = Sapphire.sign(
            Sapphire.SigningAlg.Secp256k1PrehashedKeccak256,
            _keyPair.privateKey,
            abi.encodePacked(keccak256(result)),
            ""
        );

        return PartialComputationsProof(result, signature);
    }

compute() is used by BitcoinVerifier.sol which would return the partial computions proof i.e computation result and contract signature. Both of these are expected to return in bytes. PartialComputationsProof struct is implemented as:

    struct PartialComputationsProof {
        bytes computationsResult;    @audit // both are bytes
        bytes contractSignature;
    }

_compute() has used an internal function which is overridden in BitcoinProver.sol and _compute() is implemented as:

    function _compute(bytes calldata input) internal virtual override view returns (bytes memory) {

This returns the result in bytes which is further passed as shown in below lines of code to get the signature.

        bytes memory signature = Sapphire.sign(
            Sapphire.SigningAlg.Secp256k1PrehashedKeccak256,
            _keyPair.privateKey,
@>          abi.encodePacked(keccak256(result)),
            ""
        );

result input from bytes is converted to bytes32 via encoding with keccak256, However, this is not correct.Sapphire.sign() is implemented as:

    function sign(
        SigningAlg alg,
        bytes memory secretKey,
@>        bytes memory contextOrHash,
        bytes memory message
    ) internal view returns (bytes memory signature) {
        (bool success, bytes memory sig) = SIGN_DIGEST.staticcall(
@>            abi.encode(alg, secretKey, contextOrHash, message)
        );
        require(success, "sign: failed");
        return sig;
    }

contextOrHash here as result is expected to be passed as bytes and not in bytes32 format. It should be noted that contextOrHash is further encoded via abi.encode after getting the bytes argument so passing the bytes32 argument instead of bytes will return incorrect result and sign() will throw an error "sign: failed".

Therefore, compute will always fail and return incorrect result.

Recommendations\

Consider below changes:

    function compute(bytes calldata input) public view returns (PartialComputationsProof memory) {
        bytes memory result = _compute(input);
        bytes memory signature = Sapphire.sign(
            Sapphire.SigningAlg.Secp256k1PrehashedKeccak256,
            _keyPair.privateKey,
-            abi.encodePacked(keccak256(result)),
+             result,
            ""
        );

        return PartialComputationsProof(result, signature);
    }
party-for-illuminati commented 3 days ago

The submission is invalid.

It is working properly, there are also tests for it. Please refer to Oasis Sapphire documentation: https://api.docs.oasis.io/sol/sapphire-contracts/contracts/Sapphire.sol/library.Sapphire.html#sign

It requires to provide a hash of the data to be signed/verified