diff --git a/prdoc/pr_12679.prdoc b/prdoc/pr_12679.prdoc new file mode 100644 index 000000000000..6ae099b9e746 --- /dev/null +++ b/prdoc/pr_12679.prdoc @@ -0,0 +1,20 @@ +title: '[pallet-revive] reject r/s out of range in the ecRecover precompile' + +doc: +- audience: Runtime Dev + description: | + The `0x01` ecRecover precompile only validated `v`, leaving `r` and `s` + unchecked. Depending on the `sp_io` backend, an out of range `s` can be + reduced modulo the curve order `N` instead of being rejected, so a + signature that go-ethereum rejects could recover an address here. That is a + consensus divergence, and it lets an attacker mint a second valid signature + by adding `N`, defeating malleability and replay assumptions in contracts. + + The precompile now rejects `r` or `s` equal to zero or greater than or equal + to `N` before recovery, returning the empty output (Solidity `address(0)`). + This matches go-ethereum `ValidateSignatureValues` with `homestead=false`, so + high `s` values in the range `1..N` are still accepted. + +crates: +- name: pallet-revive + bump: patch diff --git a/substrate/frame/revive/src/precompiles/builtin/ecrecover.rs b/substrate/frame/revive/src/precompiles/builtin/ecrecover.rs index 25cae2fce202..a502a03d26d0 100644 --- a/substrate/frame/revive/src/precompiles/builtin/ecrecover.rs +++ b/substrate/frame/revive/src/precompiles/builtin/ecrecover.rs @@ -22,6 +22,13 @@ use crate::{ }; use alloc::vec::Vec; use core::{marker::PhantomData, num::NonZero}; +use sp_core::U256; + +/// Order of the secp256k1 curve; valid `r` and `s` lie in `1..N`. +const SECP256K1_N: [u8; 32] = [ + 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xfe, + 0xba, 0xae, 0xdc, 0xe6, 0xaf, 0x48, 0xa0, 0x3b, 0xbf, 0xd2, 0x5e, 0x8c, 0xd0, 0x36, 0x41, 0x41, +]; pub struct EcRecover(PhantomData); @@ -54,6 +61,15 @@ impl PrimitivePrecompile for EcRecover { return Ok(Vec::new()); } + // Reject r,s outside 1..N to match go-ethereum (homestead=false); some sp_io backends + // reduce out-of-range s modulo N, diverging and allowing malleability. + let n = U256::from_big_endian(&SECP256K1_N); + let r = U256::from_big_endian(&sig[0..32]); + let s = U256::from_big_endian(&sig[32..64]); + if r.is_zero() || r >= n || s.is_zero() || s >= n { + return Ok(Vec::new()); + } + let data = match sp_io::crypto::secp256k1_ecdsa_recover(&sig, &msg) { Ok(pubkey) => { let mut address = sp_io::hashing::keccak_256(&pubkey); diff --git a/substrate/frame/revive/src/precompiles/builtin/testdata/1-ecRecover.json b/substrate/frame/revive/src/precompiles/builtin/testdata/1-ecRecover.json index b033cbbb8247..3e29cdb969d2 100644 --- a/substrate/frame/revive/src/precompiles/builtin/testdata/1-ecRecover.json +++ b/substrate/frame/revive/src/precompiles/builtin/testdata/1-ecRecover.json @@ -33,5 +33,40 @@ "Gas": 3000, "Name": "InvalidHighV-bits-3", "NoBenchmark": false + }, + { + "Input": "18c547e4f7b0f325ad1e56f57e26c745b09a3e503d86e00e5255ff7f715d3d1c000000000000000000000000000000000000000000000000000000000000001c73b1693892219d736caba55bdb67216e485557ea6b6af75f37096c9aa6a5a75ffffffffffffffffffffffffffffffffebaaedce6af48a03bbfd25e8cd0364141", + "Expected": "", + "Gas": 3000, + "Name": "SValueEqualToN", + "NoBenchmark": false + }, + { + "Input": "18c547e4f7b0f325ad1e56f57e26c745b09a3e503d86e00e5255ff7f715d3d1c000000000000000000000000000000000000000000000000000000000000001c73b1693892219d736caba55bdb67216e485557ea6b6af75f37096c9aa6a5a75ffffffffffffffffffffffffffffffffebaaedce6af48a03bbfd25e8cd0364142", + "Expected": "", + "Gas": 3000, + "Name": "SValueGreaterThanN", + "NoBenchmark": false + }, + { + "Input": "18c547e4f7b0f325ad1e56f57e26c745b09a3e503d86e00e5255ff7f715d3d1c000000000000000000000000000000000000000000000000000000000000001c73b1693892219d736caba55bdb67216e485557ea6b6af75f37096c9aa6a5a75f0000000000000000000000000000000000000000000000000000000000000000", + "Expected": "", + "Gas": 3000, + "Name": "SValueZero", + "NoBenchmark": false + }, + { + "Input": "18c547e4f7b0f325ad1e56f57e26c745b09a3e503d86e00e5255ff7f715d3d1c000000000000000000000000000000000000000000000000000000000000001cfffffffffffffffffffffffffffffffebaaedce6af48a03bbfd25e8cd0364141eeb940b1d03b21e36b0e47e79769f095fe2ab855bd91e3a38756b7d75a9c4549", + "Expected": "", + "Gas": 3000, + "Name": "RValueEqualToN", + "NoBenchmark": false + }, + { + "Input": "18c547e4f7b0f325ad1e56f57e26c745b09a3e503d86e00e5255ff7f715d3d1c000000000000000000000000000000000000000000000000000000000000001c0000000000000000000000000000000000000000000000000000000000000000eeb940b1d03b21e36b0e47e79769f095fe2ab855bd91e3a38756b7d75a9c4549", + "Expected": "", + "Gas": 3000, + "Name": "RValueZero", + "NoBenchmark": false } ]