From cfcf93bfaf6bed271cfdf7ff33db027682af0706 Mon Sep 17 00:00:00 2001 From: Toni Tabak Date: Fri, 17 May 2024 16:14:27 +0200 Subject: [PATCH] Verify message fix (#1128) * fix: verification signature with new braavos account * refactor: change ContractSpecifies name * fix: change to getContractVersion name * fix: change method name in test * fix: implement most of Penovicq comments * fix: handling when cairoversion is known and signatureveriffunctionname is unknown * fix: verifyMessageBraavos PR polution fix * chore: cleanup * chore: cleanup unrelated to the pr but next-version --------- Co-authored-by: PhilippeR26 --- src/account/default.ts | 108 +++++++++++++++++++++++++++++---------- src/account/interface.ts | 4 ++ src/provider/rpc.ts | 13 ++--- 3 files changed, 91 insertions(+), 34 deletions(-) diff --git a/src/account/default.ts b/src/account/default.ts index dc4813c67..2bd428e95 100644 --- a/src/account/default.ts +++ b/src/account/default.ts @@ -111,7 +111,7 @@ export class Account extends Provider implements AccountInterface { } /** - * Retrieves the Cairo version from the network and sets `cairoVersion` if not already set in the constructor + * Retrieves the Cairo version from the network and sets `cairoVersion` if not already set in the constructor. * @param classHash if provided detects Cairo version from classHash, otherwise from the account address */ public async getCairoVersion(classHash?: string) { @@ -541,38 +541,90 @@ export class Account extends Provider implements AccountInterface { return getMessageHash(typedData, this.address); } - public async verifyMessageHash(hash: BigNumberish, signature: Signature): Promise { - try { - const resp = await this.callContract({ - contractAddress: this.address, - entrypoint: 'isValidSignature', - calldata: CallData.compile({ - hash: toBigInt(hash).toString(), - signature: formatSignature(signature), - }), - }); - if (BigInt(resp[0]) === 0n) { - // OpenZeppelin 0.8.0 invalid signature - return false; - } - // OpenZeppelin 0.8.0, ArgentX 0.3.0 & Braavos Cairo 0 valid signature - return true; - } catch (err) { - if ( - ['argent/invalid-signature', 'is invalid, with respect to the public key'].some( - (errMessage) => (err as Error).message.includes(errMessage) - ) - ) { - // ArgentX 0.3.0 invalid signature, Braavos Cairo 0 invalid signature - return false; + public async verifyMessageHash( + hash: BigNumberish, + signature: Signature, + signatureVerificationFunctionName?: string, + signatureVerificationResponse?: { okResponse: string[]; nokResponse: string[]; error: string[] } + ): Promise { + // HOTFIX: Accounts should conform to SNIP-6 + // (https://github.com/starknet-io/SNIPs/blob/f6998f779ee2157d5e1dea36042b08062093b3c5/SNIPS/snip-6.md?plain=1#L61), + // but they don't always conform. Also, the SNIP doesn't standardize the response if the signature isn't valid. + const knownSigVerificationFName = signatureVerificationFunctionName + ? [signatureVerificationFunctionName] + : ['isValidSignature', 'is_valid_signature']; + const knownSignatureResponse = signatureVerificationResponse || { + okResponse: [ + // any non-nok response is true + ], + nokResponse: [ + '0x0', // Devnet + '0x00', // OpenZeppelin 0.7.0 to 0.9.0 invalid signature + ], + error: [ + 'argent/invalid-signature', // ArgentX 0.3.0 to 0.3.1 + 'is invalid, with respect to the public key', // OpenZeppelin until 0.6.1, Braavos 0.0.11 + 'INVALID_SIG', // Braavos 1.0.0 + ], + }; + let error: any; + + // eslint-disable-next-line no-restricted-syntax + for (const SigVerificationFName of knownSigVerificationFName) { + try { + // eslint-disable-next-line no-await-in-loop + const resp = await this.callContract({ + contractAddress: this.address, + entrypoint: SigVerificationFName, + calldata: CallData.compile({ + hash: toBigInt(hash).toString(), + signature: formatSignature(signature), + }), + }); + // Response NOK Signature + if (knownSignatureResponse.nokResponse.includes(resp[0].toString())) { + return false; + } + // Response OK Signature + // Empty okResponse assume all non-nok responses are valid signatures + // OpenZeppelin 0.7.0 to 0.9.0, ArgentX 0.3.0 to 0.3.1 & Braavos Cairo 0.0.11 to 1.0.0 valid signature + if ( + knownSignatureResponse.okResponse.length === 0 || + knownSignatureResponse.okResponse.includes(resp[0].toString()) + ) { + return true; + } + throw Error('signatureVerificationResponse Error: response is not part of known responses'); + } catch (err) { + // Known NOK Errors + if ( + knownSignatureResponse.error.some((errMessage) => + (err as Error).message.includes(errMessage) + ) + ) { + return false; + } + // Unknown Error + error = err; } - throw Error(`Signature verification request is rejected by the network: ${err}`); } + + throw Error(`Signature verification Error: ${error}`); } - public async verifyMessage(typedData: TypedData, signature: Signature): Promise { + public async verifyMessage( + typedData: TypedData, + signature: Signature, + signatureVerificationFunctionName?: string, + signatureVerificationResponse?: { okResponse: string[]; nokResponse: string[]; error: string[] } + ): Promise { const hash = await this.hashMessage(typedData); - return this.verifyMessageHash(hash, signature); + return this.verifyMessageHash( + hash, + signature, + signatureVerificationFunctionName, + signatureVerificationResponse + ); } /* diff --git a/src/account/interface.ts b/src/account/interface.ts index 6b5d1570c..66b308777 100644 --- a/src/account/interface.ts +++ b/src/account/interface.ts @@ -368,6 +368,8 @@ export abstract class AccountInterface extends ProviderInterface { * * @param typedData - JSON object to be verified * @param signature - signature of the JSON object + * @param signatureVerificationFunctionName - optional account contract verification function name override + * @param signatureVerificationResponse - optional response override { okResponse: string[]; nokResponse: string[]; error: string[] } * @returns true if the signature is valid, false otherwise * @throws {Error} if the JSON object is not a valid JSON or the signature is not a valid signature */ @@ -379,6 +381,8 @@ export abstract class AccountInterface extends ProviderInterface { * * @param hash - hash to be verified * @param signature - signature of the hash + * @param signatureVerificationFunctionName - optional account contract verification function name override + * @param signatureVerificationResponse - optional response override { okResponse: string[]; nokResponse: string[]; error: string[] } * @returns true if the signature is valid, false otherwise * @throws {Error} if the signature is not a valid signature */ diff --git a/src/provider/rpc.ts b/src/provider/rpc.ts index 68b2f5396..66ee958e9 100644 --- a/src/provider/rpc.ts +++ b/src/provider/rpc.ts @@ -1,6 +1,4 @@ -import { ProviderInterface } from './interface'; -import { LibraryError } from './errors'; -import { RpcChannel, RPC06, RPC07 } from '../channel'; +import { RPC06, RPC07, RpcChannel } from '../channel'; import { AccountInvocations, BigNumberish, @@ -8,10 +6,12 @@ import { BlockIdentifier, BlockTag, Call, + ContractClassResponse, ContractVersion, DeclareContractTransaction, DeployAccountContractTransaction, GetBlockResponse, + GetTxReceiptResponseWithoutHelper, Invocation, InvocationsDetailsWithNonce, PendingBlock, @@ -25,12 +25,13 @@ import { getEstimateFeeBulkOptions, getSimulateTransactionOptions, waitForTransactionOptions, - GetTxReceiptResponseWithoutHelper, } from '../types'; import { getAbiContractVersion } from '../utils/calldata/cairo'; import { isSierra } from '../utils/contract'; import { RPCResponseParser } from '../utils/responseParser/rpc'; -import { ReceiptTx, GetTransactionReceiptResponse } from '../utils/transactionReceipt'; +import { GetTransactionReceiptResponse, ReceiptTx } from '../utils/transactionReceipt'; +import { LibraryError } from './errors'; +import { ProviderInterface } from './interface'; export class RpcProvider implements ProviderInterface { private responseParser: RPCResponseParser; @@ -248,7 +249,7 @@ export class RpcProvider implements ProviderInterface { compiler = true, }: getContractVersionOptions = {} ): Promise { - let contractClass; + let contractClass: ContractClassResponse; if (contractAddress) { contractClass = await this.getClassAt(contractAddress, blockIdentifier); } else if (classHash) {