012.md

Tangy Hotpink Monkey

Medium

Inadequate Signature Validation Leading to Potential Replay and Unauthorized Transactions

Summary

The VerifierBase contract employs signature verification to authenticate transactions. However, the reliance on signature length checks combined with potential weaknesses in the SignatureChecker.isValidSignatureNow function could allow attackers to bypass authentication by submitting invalid signatures of the correct length.

Vulnerability Detail

• The contract uses a modifier validateAndCancel to ensure that the signature length is exactly 65 bytes, which is the standard length for ECDSA signatures. However, this check alone does not guarantee that the signature is cryptographically valid.

    modifier validateAndCancel(Common calldata common, bytes calldata signature) {
        if (common.domain != msg.sender) revert VerifierInvalidDomainError();
@=>     if (signature.length != 65) revert VerifierInvalidSignatureError();
        if (nonces[common.account][common.nonce]) revert VerifierInvalidNonceError();
        if (groups[common.account][common.group]) revert VerifierInvalidGroupError();
        if (block.timestamp >= common.expiry) revert VerifierInvalidExpiryError();

        _cancelNonce(common.account, common.nonce);

        _;
    }

• The function verifyCommon uses SignatureChecker.isValidSignatureNow to validate the signature against the provided data and signer.

    function verifyCommon(Common calldata common, bytes calldata signature)
        external
        validateAndCancel(common, signature)
    {
@=>     if (!SignatureChecker.isValidSignatureNow(common.signer, _hashTypedDataV4(CommonLib.hash(common)), signature))
            revert VerifierInvalidSignerError();
    }

• An attacker could generate random signatures with the correct length and attempt to use them in transactions. If any of these signatures are incorrectly validated as legitimate due to a flaw in the verification process, the attacker could execute unauthorized actions.

Impact

• Attackers could potentially execute transactions without possessing a valid signature, leading to unauthorized actions within the contract.
• If nonce management or signature verification is flawed, an attacker could reuse signatures to replay transactions, causing repeated actions that should only be executed once.

Code Snippet

• https://github.com/sherlock-audit/2024-08-perennial-v2-update-3/blob/main/root/contracts/verifier/VerifierBase.sol#L18-L24
• https://github.com/sherlock-audit/2024-08-perennial-v2-update-3/blob/main/root/contracts/verifier/VerifierBase.sol#L76-L86

Tool used

Manual Review

Recommendation

Add some additional checks to the signature components.

    modifier validateAndCancel(Common calldata common, bytes calldata signature) {
        if (common.domain != msg.sender) revert VerifierInvalidDomainError();
        if (signature.length != 65) revert VerifierInvalidSignatureError();
        if (nonces[common.account][common.nonce]) revert VerifierInvalidNonceError();
        if (groups[common.account][common.group]) revert VerifierInvalidGroupError();
        if (block.timestamp >= common.expiry) revert VerifierInvalidExpiryError();

        _cancelNonce(common.account, common.nonce);

        _;
    }

+   function isValidSignature(address signer, bytes32 hash, bytes memory signature) internal view returns (bool) {
+       if (signature.length != 65) return false;

+       bytes32 r;
+       bytes32 s;
+       uint8 v;

        // Extract the signature parameters
+       assembly {
+           r := mload(add(signature, 0x20))
+           s := mload(add(signature, 0x40))
+           v := byte(0, mload(add(signature, 0x60)))
+       }

        // Ensure `v` is either 27 or 28
+       if (v != 27 && v != 28) return false;

        // Ensure `s` is in the lower half order to prevent malleability
        // secp256k1n/2 is the max value for `s` to ensure it's in the lower half
+       bytes32 maxS = 0x7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF5D576E7357A4501DDFE92F46681B20A0;
+       if (uint256(s) > uint256(maxS)) return false;

        // Use SignatureChecker to verify the signature
+       return SignatureChecker.isValidSignatureNow(signer, _hashTypedDataV4(hash), signature);
+   }