From 15964ef822025898b08ffa1bc0fb681df64a6bb2 Mon Sep 17 00:00:00 2001 From: Nicholas Rodrigues Lordello Date: Tue, 19 Mar 2024 12:32:13 +0100 Subject: [PATCH] Correct EIP-7212 Behaviour --- .../contracts/interfaces/IP256Verifier.sol | 6 +-- modules/passkey/contracts/libraries/P256.sol | 3 +- .../contracts/verifiers/FCLP256Verifier.sol | 8 ++- .../test/verifiers/FCLP256Verifier.spec.ts | 54 ++++++++++++++----- 4 files changed, 51 insertions(+), 20 deletions(-) diff --git a/modules/passkey/contracts/interfaces/IP256Verifier.sol b/modules/passkey/contracts/interfaces/IP256Verifier.sol index f233e40c..40d22ecc 100644 --- a/modules/passkey/contracts/interfaces/IP256Verifier.sol +++ b/modules/passkey/contracts/interfaces/IP256Verifier.sol @@ -18,9 +18,9 @@ interface IP256Verifier { * - `input[ 96:128]`: public key x * - `input[128:160]`: public key y * - * The output is a Solidity ABI encoded boolean value indicating whether or not the signature is - * valid. Specifically, it returns 32 bytes with a value of `0x00..00` or `0x00..01` for an - * invalid or valid signature respectively. + * The output is either: + * - `abi.encode(1)` bytes for a valid signature. + * - `""` empty bytes for an invalid signature or error. * * Note that this function does not follow the Solidity ABI format (in particular, it does not * have a 4-byte selector), which is why it requires a fallback function and not regular diff --git a/modules/passkey/contracts/libraries/P256.sol b/modules/passkey/contracts/libraries/P256.sol index 2ba320a1..427ed46c 100644 --- a/modules/passkey/contracts/libraries/P256.sol +++ b/modules/passkey/contracts/libraries/P256.sol @@ -80,7 +80,8 @@ library P256 { mstore(add(input, 128), y) // Perform staticcall and check result, note that Yul evaluates expressions from right - // to left. See + // to left. See . + mstore(0, 0) success := and( and( // Return data is exactly 32-bytes long diff --git a/modules/passkey/contracts/verifiers/FCLP256Verifier.sol b/modules/passkey/contracts/verifiers/FCLP256Verifier.sol index a8491300..fb4ead1d 100644 --- a/modules/passkey/contracts/verifiers/FCLP256Verifier.sol +++ b/modules/passkey/contracts/verifiers/FCLP256Verifier.sol @@ -16,7 +16,7 @@ contract FCLP256Verifier is IP256Verifier { */ fallback(bytes calldata input) external returns (bytes memory output) { if (input.length != 160) { - return abi.encodePacked(uint256(0)); + return ""; } bytes32 message; @@ -34,6 +34,10 @@ contract FCLP256Verifier is IP256Verifier { y := calldataload(128) } - output = abi.encode(FCL_ecdsa.ecdsa_verify(message, r, s, x, y)); + if (!FCL_ecdsa.ecdsa_verify(message, r, s, x, y)) { + return ""; + } + + output = abi.encode(1); } } diff --git a/modules/passkey/test/verifiers/FCLP256Verifier.spec.ts b/modules/passkey/test/verifiers/FCLP256Verifier.spec.ts index d9c92f8b..feea2628 100644 --- a/modules/passkey/test/verifiers/FCLP256Verifier.spec.ts +++ b/modules/passkey/test/verifiers/FCLP256Verifier.spec.ts @@ -12,13 +12,9 @@ describe('FCLP256Verifier', function () { async function verifySignature(message: BytesLike, r: BigNumberish, s: BigNumberish, x: BigNumberish, y: BigNumberish) { const coder = ethers.AbiCoder.defaultAbiCoder() - const [success] = coder.decode( - ['bool'], - await verifier.fallback!.staticCall({ - data: coder.encode(['bytes32', 'uint256', 'uint256', 'uint256', 'uint256'], [message, r, s, x, y]), - }), - ) - return success + return await verifier.fallback!.staticCall({ + data: coder.encode(['bytes32', 'uint256', 'uint256', 'uint256', 'uint256'], [message, r, s, x, y]), + }) } const account = new Account() @@ -26,6 +22,9 @@ describe('FCLP256Verifier', function () { return { verifier, verifySignature, account } }) + const SUCCESS = `0x${'00'.repeat(31)}01` + const FAILURE = '0x' + it('Should return 1 on valid signature', async function () { const { verifySignature, account } = await setupTests() @@ -33,7 +32,7 @@ describe('FCLP256Verifier', function () { const { r, s } = account.sign(message) const { x, y } = account.publicKey - expect(await verifySignature(message, r, s, x, y)).to.be.true + expect(await verifySignature(message, r, s, x, y)).to.equal(SUCCESS) }) it('Should ignore signature malleability', async function () { @@ -43,18 +42,45 @@ describe('FCLP256Verifier', function () { const { r, highS } = account.sign(message) const { x, y } = account.publicKey - expect(await verifySignature(message, r, highS, x, y)).to.be.true + expect(await verifySignature(message, r, highS, x, y)).to.equal(SUCCESS) + }) + + it('Should return empty on unverified signature', async function () { + const { verifySignature, account } = await setupTests() + + const { x, y } = account.publicKey + + expect(await verifySignature(ethers.ZeroHash, 1, 2, x, y)).to.equal(FAILURE) + }) + + it('Should return empty on invalid signature parameters', async function () { + const { verifySignature, account } = await setupTests() + + const message = ethers.id('hello world') + const { r, s } = account.sign(message) + const { x, y } = account.publicKey + + // `r` and `s` must be in the range `[1, n)`, where `n` is the order of the curve. + expect(await verifySignature(message, 0, s, x, y)).to.equal(FAILURE) + expect(await verifySignature(message, r, 0, x, y)).to.equal(FAILURE) + expect(await verifySignature(message, ethers.MaxUint256, s, x, y)).to.equal(FAILURE) + expect(await verifySignature(message, r, ethers.MaxUint256, x, y)).to.equal(FAILURE) }) - it('Should return 0 on invalid signature', async function () { - const { verifySignature } = await setupTests() + it('Should return empty on invalid public key', async function () { + const { verifySignature, account } = await setupTests() + + const message = ethers.id('hello world') + const { r, s } = account.sign(message) + const { x, y } = account.publicKey - expect(await verifySignature(ethers.ZeroHash, 1, 2, 3, 4)).to.be.false + expect(await verifySignature(message, r, s, 0, y)).to.equal(FAILURE) + expect(await verifySignature(message, r, s, x, 0)).to.equal(FAILURE) }) - it('Should return 0 on invalid input', async function () { + it('Should return empty on invalid input', async function () { const { verifier } = await setupTests() - expect(await verifier.fallback!.staticCall({ data: ethers.hexlify(ethers.toUtf8Bytes('invalid input')) })).to.equal(ethers.ZeroHash) + expect(await verifier.fallback!.staticCall({ data: ethers.hexlify(ethers.toUtf8Bytes('invalid input')) })).to.equal(FAILURE) }) })