From aa293016728bf3a3890eceddb7d38af9c62470ad Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Thu, 3 Apr 2025 17:02:04 +0200 Subject: [PATCH] Fix issue with detection of RIP7212 precompile (#5620) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Arr00 <13561405+arr00@users.noreply.github.com> Co-authored-by: Ernesto García --- .changeset/mighty-melons-cheer.md | 5 + contracts/utils/cryptography/P256.sol | 42 ++++- hardhat.config.js | 1 + package-lock.json | 255 +++++++++++++------------- package.json | 2 +- test/utils/cryptography/P256.t.sol | 22 +++ test/utils/cryptography/P256.test.js | 86 ++++++--- 7 files changed, 249 insertions(+), 164 deletions(-) create mode 100644 .changeset/mighty-melons-cheer.md diff --git a/.changeset/mighty-melons-cheer.md b/.changeset/mighty-melons-cheer.md new file mode 100644 index 000000000..ca8d0df8b --- /dev/null +++ b/.changeset/mighty-melons-cheer.md @@ -0,0 +1,5 @@ +--- +'openzeppelin-solidity': patch +--- + +`P256`: Adjust precompile detection in `verifyNative` to consider empty `returndata` on invalid verification. Previously, invalid signatures would've reverted with a `MissingPrecompile` error in chains with RIP-7212 support. diff --git a/contracts/utils/cryptography/P256.sol b/contracts/utils/cryptography/P256.sol index 510eb55e7..8ec031238 100644 --- a/contracts/utils/cryptography/P256.sol +++ b/contracts/utils/cryptography/P256.sol @@ -90,10 +90,48 @@ library P256 { ) private view returns (bool valid, bool supported) { if (!_isProperSignature(r, s) || !isValidPublicKey(qx, qy)) { return (false, true); // signature is invalid, and its not because the precompile is missing + } else if (_rip7212(h, r, s, qx, qy)) { + return (true, true); // precompile is present, signature is valid + } else if ( + // Given precompiles have no bytecode (i.e. `address(0x100).code.length == 0`), we use + // a valid signature with small `r` and `s` values to check if the precompile is present. Taken from + // https://github.com/C2SP/wycheproof/blob/4672ff74d68766e7785c2cac4c597effccef2c5c/testvectors/ecdsa_secp256r1_sha256_p1363_test.json#L1173-L1204 + _rip7212( + 0xbb5a52f42f9c9261ed4361f59422a1e30036e7c32b270c8807a419feca605023, // sha256("123400") + 0x0000000000000000000000000000000000000000000000000000000000000005, + 0x0000000000000000000000000000000000000000000000000000000000000001, + 0xa71af64de5126a4a4e02b7922d66ce9415ce88a4c9d25514d91082c8725ac957, + 0x5d47723c8fbe580bb369fec9c2665d8e30a435b9932645482e7c9f11e872296b + ) + ) { + return (false, true); // precompile is present, signature is invalid + } else { + return (false, false); // precompile is absent } + } - (bool success, bytes memory returndata) = address(0x100).staticcall(abi.encode(h, r, s, qx, qy)); - return (success && returndata.length == 0x20) ? (abi.decode(returndata, (bool)), true) : (false, false); + /** + * @dev Low level helper for {_tryVerifyNative}. Calls the precompile and checks if there is a return value. + */ + function _rip7212(bytes32 h, bytes32 r, bytes32 s, bytes32 qx, bytes32 qy) private view returns (bool isValid) { + assembly ("memory-safe") { + // Use the free memory pointer without updating it at the end of the function + let ptr := mload(0x40) + mstore(ptr, h) + mstore(add(ptr, 0x20), r) + mstore(add(ptr, 0x40), s) + mstore(add(ptr, 0x60), qx) + mstore(add(ptr, 0x80), qy) + // RIP-7212 precompiles return empty bytes when an invalid signature is passed, making it impossible + // to distinguish the presence of the precompile. Custom precompile implementations may decide to + // return `bytes32(0)` (i.e. false) without developers noticing, so we decide to evaluate the return value + // without expanding memory using scratch space. + mstore(0x00, 0) // zero out scratch space in case the precompile doesn't return anything + if iszero(staticcall(gas(), 0x100, ptr, 0xa0, 0x00, 0x20)) { + invalid() + } + isValid := mload(0x00) + } } /** diff --git a/hardhat.config.js b/hardhat.config.js index b4b8d630f..30c19ca6d 100644 --- a/hardhat.config.js +++ b/hardhat.config.js @@ -102,6 +102,7 @@ module.exports = { // we rely on the `code-size` compiler warning, that will cause a compilation error. allowUnlimitedContractSize: true, initialBaseFeePerGas: argv.coverage ? 0 : undefined, + enableRip7212: true, }, }, exposed: { diff --git a/package-lock.json b/package-lock.json index a7f7af145..2b2d87fae 100644 --- a/package-lock.json +++ b/package-lock.json @@ -28,7 +28,7 @@ "glob": "^11.0.0", "globals": "^16.0.0", "graphlib": "^2.1.8", - "hardhat": "^2.22.2", + "hardhat": "^2.22.7", "hardhat-exposed": "^0.3.15", "hardhat-gas-reporter": "^2.1.0", "hardhat-ignore-warnings": "^0.2.11", @@ -1553,53 +1553,90 @@ } }, "node_modules/@nomicfoundation/edr": { - "version": "0.3.3", - "resolved": "https://registry.npmjs.org/@nomicfoundation/edr/-/edr-0.3.3.tgz", - "integrity": "sha512-zP+e+3B1nEUx6bW5BPnIzCQbkhmYfdMBJdiVggTqqTfAA82sOkdOG7wsOMcz5qF3fYfx/irNRM1kgc9HVFIbpQ==", + "version": "0.5.2", + "resolved": "https://registry.npmjs.org/@nomicfoundation/edr/-/edr-0.5.2.tgz", + "integrity": "sha512-hW/iLvUQZNTVjFyX/I40rtKvvDOqUEyIi96T28YaLfmPL+3LW2lxmYLUXEJ6MI14HzqxDqrLyhf6IbjAa2r3Dw==", "dev": true, + "license": "MIT", + "dependencies": { + "@nomicfoundation/edr-darwin-arm64": "0.5.2", + "@nomicfoundation/edr-darwin-x64": "0.5.2", + "@nomicfoundation/edr-linux-arm64-gnu": "0.5.2", + "@nomicfoundation/edr-linux-arm64-musl": "0.5.2", + "@nomicfoundation/edr-linux-x64-gnu": "0.5.2", + "@nomicfoundation/edr-linux-x64-musl": "0.5.2", + "@nomicfoundation/edr-win32-x64-msvc": "0.5.2" + }, + "engines": { + "node": ">= 18" + } + }, + "node_modules/@nomicfoundation/edr-darwin-arm64": { + "version": "0.5.2", + "resolved": "https://registry.npmjs.org/@nomicfoundation/edr-darwin-arm64/-/edr-darwin-arm64-0.5.2.tgz", + "integrity": "sha512-Gm4wOPKhbDjGTIRyFA2QUAPfCXA1AHxYOKt3yLSGJkQkdy9a5WW+qtqKeEKHc/+4wpJSLtsGQfpzyIzggFfo/A==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">= 18" + } + }, + "node_modules/@nomicfoundation/edr-darwin-x64": { + "version": "0.5.2", + "resolved": "https://registry.npmjs.org/@nomicfoundation/edr-darwin-x64/-/edr-darwin-x64-0.5.2.tgz", + "integrity": "sha512-ClyABq2dFCsrYEED3/UIO0c7p4H1/4vvlswFlqUyBpOkJccr75qIYvahOSJRM62WgUFRhbSS0OJXFRwc/PwmVg==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">= 18" + } + }, + "node_modules/@nomicfoundation/edr-linux-arm64-gnu": { + "version": "0.5.2", + "resolved": "https://registry.npmjs.org/@nomicfoundation/edr-linux-arm64-gnu/-/edr-linux-arm64-gnu-0.5.2.tgz", + "integrity": "sha512-HWMTVk1iOabfvU2RvrKLDgtFjJZTC42CpHiw2h6rfpsgRqMahvIlx2jdjWYzFNy1jZKPTN1AStQ/91MRrg5KnA==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">= 18" + } + }, + "node_modules/@nomicfoundation/edr-linux-arm64-musl": { + "version": "0.5.2", + "resolved": "https://registry.npmjs.org/@nomicfoundation/edr-linux-arm64-musl/-/edr-linux-arm64-musl-0.5.2.tgz", + "integrity": "sha512-CwsQ10xFx/QAD5y3/g5alm9+jFVuhc7uYMhrZAu9UVF+KtVjeCvafj0PaVsZ8qyijjqVuVsJ8hD1x5ob7SMcGg==", + "dev": true, + "license": "MIT", "engines": { "node": ">= 18" - }, - "optionalDependencies": { - "@nomicfoundation/edr-darwin-arm64": "0.3.3", - "@nomicfoundation/edr-darwin-x64": "0.3.3", - "@nomicfoundation/edr-linux-arm64-gnu": "0.3.3", - "@nomicfoundation/edr-linux-arm64-musl": "0.3.3", - "@nomicfoundation/edr-linux-x64-gnu": "0.3.3", - "@nomicfoundation/edr-linux-x64-musl": "0.3.3", - "@nomicfoundation/edr-win32-arm64-msvc": "0.3.3", - "@nomicfoundation/edr-win32-ia32-msvc": "0.3.3", - "@nomicfoundation/edr-win32-x64-msvc": "0.3.3" } }, "node_modules/@nomicfoundation/edr-linux-x64-gnu": { - "version": "0.3.3", - "resolved": "https://registry.npmjs.org/@nomicfoundation/edr-linux-x64-gnu/-/edr-linux-x64-gnu-0.3.3.tgz", - "integrity": "sha512-xElOs1U+E6lBLtv1mnJ+E8nr2MxZgKiLo8bZAgBboy9odYtmkDVwhMjtsFKSuZbGxFtsSyGRT4cXw3JAbtUDeA==", - "cpu": [ - "x64" - ], + "version": "0.5.2", + "resolved": "https://registry.npmjs.org/@nomicfoundation/edr-linux-x64-gnu/-/edr-linux-x64-gnu-0.5.2.tgz", + "integrity": "sha512-CWVCEdhWJ3fmUpzWHCRnC0/VLBDbqtqTGTR6yyY1Ep3S3BOrHEAvt7h5gx85r2vLcztisu2vlDq51auie4IU1A==", "dev": true, - "optional": true, - "os": [ - "linux" - ], + "license": "MIT", "engines": { "node": ">= 18" } }, "node_modules/@nomicfoundation/edr-linux-x64-musl": { - "version": "0.3.3", - "resolved": "https://registry.npmjs.org/@nomicfoundation/edr-linux-x64-musl/-/edr-linux-x64-musl-0.3.3.tgz", - "integrity": "sha512-2Fe6gwm1RAGQ/PfMYiaSba2OrFp8zzYWh+am9lYObOFjV9D+A1zhIzfy0UC74glPks5eV8eY4pBPrVR042m2Nw==", - "cpu": [ - "x64" - ], + "version": "0.5.2", + "resolved": "https://registry.npmjs.org/@nomicfoundation/edr-linux-x64-musl/-/edr-linux-x64-musl-0.5.2.tgz", + "integrity": "sha512-+aJDfwhkddy2pP5u1ISg3IZVAm0dO836tRlDTFWtvvSMQ5hRGqPcWwlsbobhDQsIxhPJyT7phL0orCg5W3WMeA==", "dev": true, - "optional": true, - "os": [ - "linux" - ], + "license": "MIT", + "engines": { + "node": ">= 18" + } + }, + "node_modules/@nomicfoundation/edr-win32-x64-msvc": { + "version": "0.5.2", + "resolved": "https://registry.npmjs.org/@nomicfoundation/edr-win32-x64-msvc/-/edr-win32-x64-msvc-0.5.2.tgz", + "integrity": "sha512-CcvvuA3sAv7liFNPsIR/68YlH6rrybKzYttLlMr80d4GKJjwJ5OKb3YgE6FdZZnOfP19HEHhsLcE0DPLtY3r0w==", + "dev": true, + "license": "MIT", "engines": { "node": ">= 18" } @@ -3494,7 +3531,8 @@ "version": "1.2.9", "resolved": "https://registry.npmjs.org/command-exists/-/command-exists-1.2.9.tgz", "integrity": "sha512-LTQ/SGc+s0Xc0Fu5WaKnR0YiygZkm9eKFvyS+fRsU7/ZWFF8ykFM6Pc9aCVf1+xasOOZpO3BAVgVrKvsqKHV7w==", - "dev": true + "dev": true, + "license": "MIT" }, "node_modules/commander": { "version": "10.0.1", @@ -5506,14 +5544,15 @@ } }, "node_modules/hardhat": { - "version": "2.22.2", - "resolved": "https://registry.npmjs.org/hardhat/-/hardhat-2.22.2.tgz", - "integrity": "sha512-0xZ7MdCZ5sJem4MrvpQWLR3R3zGDoHw5lsR+pBFimqwagimIOn3bWuZv69KA+veXClwI1s/zpqgwPwiFrd4Dxw==", + "version": "2.22.7", + "resolved": "https://registry.npmjs.org/hardhat/-/hardhat-2.22.7.tgz", + "integrity": "sha512-nrXQAl+qUr75TsCLDo8P41YXLc+5U7qQMMCIrbbmy1/uQaVPncdjDrD5BR0CENvHRj7EBqO+JkofpozXoIfJKg==", "dev": true, + "license": "MIT", "dependencies": { "@ethersproject/abi": "^5.1.2", "@metamask/eth-sig-util": "^4.0.0", - "@nomicfoundation/edr": "^0.3.1", + "@nomicfoundation/edr": "^0.5.0", "@nomicfoundation/ethereumjs-common": "4.0.4", "@nomicfoundation/ethereumjs-tx": "5.0.4", "@nomicfoundation/ethereumjs-util": "9.0.4", @@ -5547,7 +5586,7 @@ "raw-body": "^2.4.1", "resolve": "1.17.0", "semver": "^6.3.0", - "solc": "0.7.3", + "solc": "0.8.26", "source-map-support": "^0.5.13", "stacktrace-parser": "^0.1.10", "tsort": "0.0.1", @@ -5965,12 +6004,6 @@ "integrity": "sha512-5tK7EtrZ0N+OLFMthtqOj4fI2Jeb88C4CAZPu25LDVUgXJ0A3Js4PMGqrn0JU1W0Mh1/Z8wZzYPxqUrXeBboCQ==", "dev": true }, - "node_modules/hardhat/node_modules/commander": { - "version": "3.0.2", - "resolved": "https://registry.npmjs.org/commander/-/commander-3.0.2.tgz", - "integrity": "sha512-Gar0ASD4BDyKC4hl4DwHqDrmvjoxWKZigVnAbn5H1owvm4CxCPdb0HQDehwNYMJpla5+M2tPmPARzhtYuwpHow==", - "dev": true - }, "node_modules/hardhat/node_modules/ethereum-cryptography": { "version": "1.2.0", "resolved": "https://registry.npmjs.org/ethereum-cryptography/-/ethereum-cryptography-1.2.0.tgz", @@ -5999,7 +6032,9 @@ "version": "7.2.0", "resolved": "https://registry.npmjs.org/glob/-/glob-7.2.0.tgz", "integrity": "sha512-lmLf6gtyrPq8tTjSmrO94wBeQbFR3HbLHbuyD69wuyQkImp2hWqMGB47OX65FBkPffO641IP9jWa1z4ivqG26Q==", + "deprecated": "Glob versions prior to v9 are no longer supported", "dev": true, + "license": "ISC", "dependencies": { "fs.realpath": "^1.0.0", "inflight": "^1.0.4", @@ -6015,15 +6050,6 @@ "url": "https://github.com/sponsors/isaacs" } }, - "node_modules/hardhat/node_modules/jsonfile": { - "version": "2.4.0", - "resolved": "https://registry.npmjs.org/jsonfile/-/jsonfile-2.4.0.tgz", - "integrity": "sha512-PKllAqbgLgxHaj8TElYymKCAgrASebJrWpTnEkOaTowt23VKXXN0sUeriJ+eh7y6ufb/CC5ap11pz71/cM0hUw==", - "dev": true, - "optionalDependencies": { - "graceful-fs": "^4.1.6" - } - }, "node_modules/hardhat/node_modules/locate-path": { "version": "2.0.0", "resolved": "https://registry.npmjs.org/locate-path/-/locate-path-2.0.0.tgz", @@ -6079,27 +6105,6 @@ "node": ">=4" } }, - "node_modules/hardhat/node_modules/require-from-string": { - "version": "2.0.2", - "resolved": "https://registry.npmjs.org/require-from-string/-/require-from-string-2.0.2.tgz", - "integrity": "sha512-Xf0nWe6RseziFMu+Ap9biiUbmplq6S9/p+7w7YXP/JBHhrUDDUhwa+vANyubuqfZWTveU//DYVGsDG7RKL/vEw==", - "dev": true, - "engines": { - "node": ">=0.10.0" - } - }, - "node_modules/hardhat/node_modules/rimraf": { - "version": "2.7.1", - "resolved": "https://registry.npmjs.org/rimraf/-/rimraf-2.7.1.tgz", - "integrity": "sha512-uWjbaKIK3T1OSVptzX7Nl6PvQ3qAGtKEtVRjRuazjfL3Bx5eI409VZSqgND+4UNnmzLVdPj9FqFJNPqBZFve4w==", - "dev": true, - "dependencies": { - "glob": "^7.1.3" - }, - "bin": { - "rimraf": "bin.js" - } - }, "node_modules/hardhat/node_modules/semver": { "version": "6.3.1", "resolved": "https://registry.npmjs.org/semver/-/semver-6.3.1.tgz", @@ -6109,51 +6114,6 @@ "semver": "bin/semver.js" } }, - "node_modules/hardhat/node_modules/solc": { - "version": "0.7.3", - "resolved": "https://registry.npmjs.org/solc/-/solc-0.7.3.tgz", - "integrity": "sha512-GAsWNAjGzIDg7VxzP6mPjdurby3IkGCjQcM8GFYZT6RyaoUZKmMU6Y7YwG+tFGhv7dwZ8rmR4iwFDrrD99JwqA==", - "dev": true, - "dependencies": { - "command-exists": "^1.2.8", - "commander": "3.0.2", - "follow-redirects": "^1.12.1", - "fs-extra": "^0.30.0", - "js-sha3": "0.8.0", - "memorystream": "^0.3.1", - "require-from-string": "^2.0.0", - "semver": "^5.5.0", - "tmp": "0.0.33" - }, - "bin": { - "solcjs": "solcjs" - }, - "engines": { - "node": ">=8.0.0" - } - }, - "node_modules/hardhat/node_modules/solc/node_modules/fs-extra": { - "version": "0.30.0", - "resolved": "https://registry.npmjs.org/fs-extra/-/fs-extra-0.30.0.tgz", - "integrity": "sha512-UvSPKyhMn6LEd/WpUaV9C9t3zATuqoqfWc3QdPhPLb58prN9tqYPlPWi8Krxi44loBoUzlobqZ3+8tGpxxSzwA==", - "dev": true, - "dependencies": { - "graceful-fs": "^4.1.2", - "jsonfile": "^2.1.0", - "klaw": "^1.0.0", - "path-is-absolute": "^1.0.0", - "rimraf": "^2.2.8" - } - }, - "node_modules/hardhat/node_modules/solc/node_modules/semver": { - "version": "5.7.2", - "resolved": "https://registry.npmjs.org/semver/-/semver-5.7.2.tgz", - "integrity": "sha512-cBznnQ9KjJqU67B52RMC65CMarK2600WFnbkcaiwWq3xy/5haFJlshgnpjovMVJ+Hff49d8GEn0b87C5pDQ10g==", - "dev": true, - "bin": { - "semver": "bin/semver" - } - }, "node_modules/hardhat/node_modules/undici": { "version": "5.28.4", "resolved": "https://registry.npmjs.org/undici/-/undici-5.28.4.tgz", @@ -7002,15 +6962,6 @@ "node": ">=0.10.0" } }, - "node_modules/klaw": { - "version": "1.3.1", - "resolved": "https://registry.npmjs.org/klaw/-/klaw-1.3.1.tgz", - "integrity": "sha512-TED5xi9gGQjGpNnvRWknrwAB1eL5GciPfVFOt3Vk1OJCVDQbzuSfrF3hkUQKlsgKrG1F+0t5W0m+Fje1jIt8rw==", - "dev": true, - "optionalDependencies": { - "graceful-fs": "^4.1.9" - } - }, "node_modules/kleur": { "version": "4.1.5", "resolved": "https://registry.npmjs.org/kleur/-/kleur-4.1.5.tgz", @@ -9909,6 +9860,48 @@ "node": ">=8" } }, + "node_modules/solc": { + "version": "0.8.26", + "resolved": "https://registry.npmjs.org/solc/-/solc-0.8.26.tgz", + "integrity": "sha512-yiPQNVf5rBFHwN6SIf3TUUvVAFKcQqmSUFeq+fb6pNRCo0ZCgpYOZDi3BVoezCPIAcKrVYd/qXlBLUP9wVrZ9g==", + "dev": true, + "license": "MIT", + "dependencies": { + "command-exists": "^1.2.8", + "commander": "^8.1.0", + "follow-redirects": "^1.12.1", + "js-sha3": "0.8.0", + "memorystream": "^0.3.1", + "semver": "^5.5.0", + "tmp": "0.0.33" + }, + "bin": { + "solcjs": "solc.js" + }, + "engines": { + "node": ">=10.0.0" + } + }, + "node_modules/solc/node_modules/commander": { + "version": "8.3.0", + "resolved": "https://registry.npmjs.org/commander/-/commander-8.3.0.tgz", + "integrity": "sha512-OkTL9umf+He2DZkUq8f8J9of7yL6RJKI24dVITBmNfZBmri9zYZQrKkuXiKhyfPSu8tUhnVBB1iKXevvnlR4Ww==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">= 12" + } + }, + "node_modules/solc/node_modules/semver": { + "version": "5.7.2", + "resolved": "https://registry.npmjs.org/semver/-/semver-5.7.2.tgz", + "integrity": "sha512-cBznnQ9KjJqU67B52RMC65CMarK2600WFnbkcaiwWq3xy/5haFJlshgnpjovMVJ+Hff49d8GEn0b87C5pDQ10g==", + "dev": true, + "license": "ISC", + "bin": { + "semver": "bin/semver" + } + }, "node_modules/solhint": { "version": "5.0.0", "resolved": "https://registry.npmjs.org/solhint/-/solhint-5.0.0.tgz", diff --git a/package.json b/package.json index d2befe347..8282c3843 100644 --- a/package.json +++ b/package.json @@ -70,7 +70,7 @@ "glob": "^11.0.0", "globals": "^16.0.0", "graphlib": "^2.1.8", - "hardhat": "^2.22.2", + "hardhat": "^2.22.7", "hardhat-exposed": "^0.3.15", "hardhat-gas-reporter": "^2.1.0", "hardhat-ignore-warnings": "^0.2.11", diff --git a/test/utils/cryptography/P256.t.sol b/test/utils/cryptography/P256.t.sol index d70cdf1bf..768f9087f 100644 --- a/test/utils/cryptography/P256.t.sol +++ b/test/utils/cryptography/P256.t.sol @@ -6,6 +6,7 @@ import {Test} from "forge-std/Test.sol"; import {P256} from "@openzeppelin/contracts/utils/cryptography/P256.sol"; import {Math} from "@openzeppelin/contracts/utils/math/Math.sol"; +import {Errors} from "@openzeppelin/contracts/utils/Errors.sol"; contract P256Test is Test { /// forge-config: default.fuzz.runs = 512 @@ -31,6 +32,22 @@ contract P256Test is Test { assertTrue((qx0 == bytes32(x) && qy0 == bytes32(y)) || (qx1 == bytes32(x) && qy1 == bytes32(y))); } + function testVerifyNativeUnsupportedRIP7212(bytes32 digest, uint256 seed) public { + // By default, the precompile at address 0x100 is not supported. + + uint256 privateKey = _asPrivateKey(seed); + + (uint256 x, uint256 y) = vm.publicKeyP256(privateKey); + (bytes32 r, bytes32 s) = vm.signP256(privateKey, digest); + s = _ensureLowerS(s); + + (bool success, bytes memory returndata) = address(this).call( + abi.encodeCall(P256Test.verifyNative, (digest, r, s, bytes32(x), bytes32(y))) + ); + assertFalse(success); + assertEq(returndata, abi.encodeWithSelector(Errors.MissingPrecompile.selector, address(0x100))); + } + function _asPrivateKey(uint256 seed) private pure returns (uint256) { return bound(seed, 1, P256.N - 1); } @@ -41,4 +58,9 @@ contract P256Test is Test { return _s > P256.N / 2 ? bytes32(P256.N - _s) : s; } } + + // See https://github.com/foundry-rs/foundry/issues/10237 + function verifyNative(bytes32 digest, bytes32 r, bytes32 s, bytes32 x, bytes32 y) external view { + P256.verifyNative(digest, r, s, x, y); + } } diff --git a/test/utils/cryptography/P256.test.js b/test/utils/cryptography/P256.test.js index b9655cad3..a75d527be 100644 --- a/test/utils/cryptography/P256.test.js +++ b/test/utils/cryptography/P256.test.js @@ -44,26 +44,52 @@ describe('P256', function () { }); it('verify valid signature', async function () { - expect(await this.mock.$verify(this.messageHash, ...this.signature, ...this.publicKey)).to.be.true; - expect(await this.mock.$verifySolidity(this.messageHash, ...this.signature, ...this.publicKey)).to.be.true; - await expect(this.mock.$verifyNative(this.messageHash, ...this.signature, ...this.publicKey)) - .to.be.revertedWithCustomError(this.mock, 'MissingPrecompile') - .withArgs('0x0000000000000000000000000000000000000100'); + await expect(this.mock.$verify(this.messageHash, ...this.signature, ...this.publicKey)).to.eventually.be.true; + await expect(this.mock.$verifySolidity(this.messageHash, ...this.signature, ...this.publicKey)).to.eventually.be + .true; + await expect(this.mock.$verifyNative(this.messageHash, ...this.signature, ...this.publicKey)).to.eventually.be + .true; + }); + + it('verify improper signature', async function () { + const signature = this.signature; + this.signature[0] = ethers.toBeHex(N, 0x20); // r = N + await expect(this.mock.$verify(this.messageHash, ...signature, ...this.publicKey)).to.eventually.be.false; + await expect(this.mock.$verifySolidity(this.messageHash, ...signature, ...this.publicKey)).to.eventually.be.false; + await expect(this.mock.$verifyNative(this.messageHash, ...signature, ...this.publicKey)).to.eventually.be.false; }); it('recover public key', async function () { - expect(await this.mock.$recovery(this.messageHash, this.recovery, ...this.signature)).to.deep.equal( + await expect(this.mock.$recovery(this.messageHash, this.recovery, ...this.signature)).to.eventually.deep.equal( this.publicKey, ); }); + it('recovers (0,0) for invalid recovery bit', async function () { + await expect(this.mock.$recovery(this.messageHash, 2, ...this.signature)).to.eventually.deep.equal([ + ethers.ZeroHash, + ethers.ZeroHash, + ]); + }); + + it('recovers (0,0) for improper signature', async function () { + const signature = this.signature; + this.signature[0] = ethers.toBeHex(N, 0x20); // r = N + await expect(this.mock.$recovery(this.messageHash, this.recovery, ...signature)).to.eventually.deep.equal([ + ethers.ZeroHash, + ethers.ZeroHash, + ]); + }); + it('reject signature with flipped public key coordinates ([x,y] >> [y,x])', async function () { // flip public key this.publicKey.reverse(); - expect(await this.mock.$verify(this.messageHash, ...this.signature, ...this.publicKey)).to.be.false; - expect(await this.mock.$verifySolidity(this.messageHash, ...this.signature, ...this.publicKey)).to.be.false; - expect(await this.mock.$verifyNative(this.messageHash, ...this.signature, ...this.publicKey)).to.be.false; // Flipped public key is not in the curve + await expect(this.mock.$verify(this.messageHash, ...this.signature, ...this.publicKey)).to.eventually.be.false; + await expect(this.mock.$verifySolidity(this.messageHash, ...this.signature, ...this.publicKey)).to.eventually.be + .false; + await expect(this.mock.$verifyNative(this.messageHash, ...this.signature, ...this.publicKey)).to.eventually.be + .false; }); it('reject signature with flipped signature values ([r,s] >> [s,r])', async function () { @@ -81,42 +107,42 @@ describe('P256', function () { ]; // Make sure it works - expect(await this.mock.$verify(this.messageHash, ...this.signature, ...this.publicKey)).to.be.true; + await expect(this.mock.$verify(this.messageHash, ...this.signature, ...this.publicKey)).to.eventually.be.true; // Flip signature this.signature.reverse(); - expect(await this.mock.$verify(this.messageHash, ...this.signature, ...this.publicKey)).to.be.false; - expect(await this.mock.$verifySolidity(this.messageHash, ...this.signature, ...this.publicKey)).to.be.false; - await expect(this.mock.$verifyNative(this.messageHash, ...this.signature, ...this.publicKey)) - .to.be.revertedWithCustomError(this.mock, 'MissingPrecompile') - .withArgs('0x0000000000000000000000000000000000000100'); - expect(await this.mock.$recovery(this.messageHash, this.recovery, ...this.signature)).to.not.deep.equal( - this.publicKey, - ); + await expect(this.mock.$verify(this.messageHash, ...this.signature, ...this.publicKey)).to.eventually.be.false; + await expect(this.mock.$verifySolidity(this.messageHash, ...this.signature, ...this.publicKey)).to.eventually.be + .false; + await expect(this.mock.$verifyNative(this.messageHash, ...this.signature, ...this.publicKey)).to.eventually.be + .false; + await expect( + this.mock.$recovery(this.messageHash, this.recovery, ...this.signature), + ).to.eventually.not.deep.equal(this.publicKey); }); it('reject signature with invalid message hash', async function () { // random message hash this.messageHash = ethers.hexlify(ethers.randomBytes(32)); - expect(await this.mock.$verify(this.messageHash, ...this.signature, ...this.publicKey)).to.be.false; - expect(await this.mock.$verifySolidity(this.messageHash, ...this.signature, ...this.publicKey)).to.be.false; - await expect(this.mock.$verifyNative(this.messageHash, ...this.signature, ...this.publicKey)) - .to.be.revertedWithCustomError(this.mock, 'MissingPrecompile') - .withArgs('0x0000000000000000000000000000000000000100'); - expect(await this.mock.$recovery(this.messageHash, this.recovery, ...this.signature)).to.not.deep.equal( - this.publicKey, - ); + await expect(this.mock.$verify(this.messageHash, ...this.signature, ...this.publicKey)).to.eventually.be.false; + await expect(this.mock.$verifySolidity(this.messageHash, ...this.signature, ...this.publicKey)).to.eventually.be + .false; + await expect(this.mock.$verifyNative(this.messageHash, ...this.signature, ...this.publicKey)).to.eventually.be + .false; + await expect( + this.mock.$recovery(this.messageHash, this.recovery, ...this.signature), + ).to.eventually.not.deep.equal(this.publicKey); }); it('fail to recover signature with invalid recovery bit', async function () { // flip recovery bit this.recovery = 1 - this.recovery; - expect(await this.mock.$recovery(this.messageHash, this.recovery, ...this.signature)).to.not.deep.equal( - this.publicKey, - ); + await expect( + this.mock.$recovery(this.messageHash, this.recovery, ...this.signature), + ).to.eventually.not.deep.equal(this.publicKey); }); }); @@ -148,7 +174,7 @@ describe('P256', function () { const messageHash = ethers.sha256('0x' + msg); // check verify - expect(await this.mock.$verify(messageHash, r, s, x, y)).to.equal(result == 'valid'); + await expect(this.mock.$verify(messageHash, r, s, x, y)).to.eventually.equal(result == 'valid'); }); } }