fix(zk-verifier): bound deserialization input and stop circuit-id aliasing

frame/shielded-pool/CHANGELOG.md

@@ -29,6 +29,13 @@ All notable changes to `pallet-shielded-pool` will be documented in this file.
   is immutable so the result is byte-identical to what was stored. The active
   tree is untouched: still 20 point reads and zero hashes.
 
+### Fixed
+- `hash_pair_poseidon` clamps its output copy instead of slicing `&bytes[..32]`
+  raw. BN254 `Fr` always yields 32 bytes, so the clamp never binds today — but
+  this runs on the block-import path, where slicing past the end panics the node
+  rather than failing a call. `recipient_to_field` was already written this way;
+  the two now match.
+
 ### Removed
 - **Minimum shield amount.** The `MinShieldAmount` Config constant (1 ORB in the
   runtime) and the `AmountTooSmall` error are gone. `shield` now accepts any

frame/shielded-pool/src/merkle/hashing.rs

@@ -54,10 +54,15 @@ pub fn hash_pair_poseidon(left: &[u8; 32], right: &[u8; 32]) -> [u8; 32] {
 
 	let hash_fr = hasher.hash_2([FieldElement::new(left_fr), FieldElement::new(right_fr)]);
 
+	// BN254 `Fr` always yields 32 bytes, so the clamp never binds today. It is
+	// here because this runs on the block-import path, where slicing past the
+	// end would panic the node rather than fail a call — the same reason
+	// `recipient_to_field` is written this way.
 	let mut hash_bytes = [0u8; 32];
 	let bigint = hash_fr.inner().into_bigint();
 	let bytes = bigint.to_bytes_le();
-	hash_bytes.copy_from_slice(&bytes[..32]);
+	let n = bytes.len().min(32);
+	hash_bytes[..n].copy_from_slice(&bytes[..n]);
 	hash_bytes
 }
 

frame/zk-verifier/CHANGELOG.md

@@ -4,6 +4,49 @@ All notable changes to this pallet are documented here.
 
 ---
 
+## [0.12.0] - 2026-08-07
+
+### Security
+
+- **Circuit ids that do not fit a `u8` are rejected instead of truncated.**
+  `expected_public_inputs` takes a `u8`, and `ensure_vk_arity` reached it through
+  `circuit_id.0 as u8` — so id 257 aliased onto 1 and a key was validated against
+  TRANSFER's arity, then stored under an id no lookup could reach. Now guarded
+  with `u8::try_from`, the same way `purge_circuit` already guarded the same
+  table. Root-gated, so this is an operator-error amplifier rather than an attack
+  primitive; what made it worth closing is that it failed silently.
+
+  Ids inside `u8` but outside the known table are unaffected: they carry no
+  expected arity, so only "deserializes as a BN254 key" applies to them.
+
+- **Genesis validates its keys.** `build` checked length alone, so a well-sized
+  but meaningless key was stored and the chain only discovered it when the first
+  real proof failed to verify — at which point nothing distinguishes a bad key
+  from a bad proof. Genesis now routes through the same `ensure_vk_arity` as
+  `register_verification_key`, turning that into a chain that refuses to start.
+
+  **Breaking for genesis configs carrying placeholder keys.** Two of this
+  pallet's own tests were doing exactly that (`vec![0xCCu8; 300]`) and now fail;
+  they were updated to real keys.
+
+### Fixed
+
+- A test helper's doc claimed the TRANSFER circuit has arity 5 while the constant
+  it reads is 7. Rewritten to point at the constant instead of restating the
+  number, which is how the two drifted apart.
+
+### Notes
+
+- The aliasing guard was verified by reverting it and confirming
+  `register_vk_rejects_circuit_id_that_would_alias` fails.
+- A dev-node E2E (`ts-tests/node/zk-verifier-input-bounds.test.cjs`, 9/9) covers
+  the live path: genesis keys pass the new check at startup, all three real VKs
+  register through `setup-dev-local.sh`, an 8 KiB filler key is refused with
+  `InvalidVerificationKey`, id 257 is refused and stores nothing, and the chain
+  keeps producing blocks throughout.
+
+---
+
 ## [0.11.0] - 2026-08-04
 
 ### Removed

frame/zk-verifier/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "pallet-zk-verifier"
-version = "0.11.0"
+version = "0.12.0"
 description = "Zero-Knowledge proof verification pallet for Orbinum"
 authors = ["Orbinum Team"]
 license = "GPL-3.0-or-later"

frame/zk-verifier/src/lib.rs

@@ -147,6 +147,9 @@ pub mod pallet {
 					.try_into()
 					.expect("Genesis VK exceeds maximum size (8 KB)");
 
+				Pallet::<T>::ensure_vk_arity(*circuit_id, key_data.as_slice())
+					.expect("Genesis VK must deserialize and match its circuit's arity");
+
 				let hash = sp_io::hashing::blake2_256(key_data.as_slice());
 				VerificationKeys::<T>::insert(
 					circuit_id,
@@ -612,18 +615,25 @@ pub mod pallet {
 	}
 
 	impl<T: Config> Pallet<T> {
-		/// Verify the VK deserializes as a BN254 Groth16 key and, for known circuits,
-		/// that its arity matches. Unknown circuits are only checked to deserialize.
+		/// Verify the VK deserializes as a BN254 Groth16 key and that its arity
+		/// matches the circuit it is being stored under.
+		///
+		/// Only ids in the known table carry an expected arity; the rest are
+		/// checked to deserialize and nothing more. An id that does not fit a
+		/// `u8` is rejected outright rather than truncated — `as u8` would alias
+		/// 257 onto 1 and silently validate a key against the wrong circuit's
+		/// arity. `purge_circuit` guards the same lookup the same way.
 		fn ensure_vk_arity(circuit_id: CircuitId, key_data: &[u8]) -> DispatchResult {
 			use orbinum_zk_verifier::{VerifyingKey, expected_public_inputs};
 
+			let id = u8::try_from(circuit_id.0).map_err(|_| Error::<T>::InvalidVerificationKey)?;
+
 			let vk = VerifyingKey::new(key_data.to_vec());
 			let arity = vk
 				.num_public_inputs()
 				.map_err(|_| Error::<T>::InvalidVerificationKey)?;
 
-			// circuit ids fit in u8 for the known set.
-			if let Some(expected) = expected_public_inputs(circuit_id.0 as u8) {
+			if let Some(expected) = expected_public_inputs(id) {
 				ensure!(arity == expected, Error::<T>::InvalidVerificationKey);
 			}
 			Ok(())
@@ -714,7 +724,6 @@ mod tests {
 			.expect("serialized VK fits in 8192 bytes")
 	}
 
-	/// VK for the TRANSFER circuit (arity 5) — the default used by most tests.
 	fn vk_bytes() -> BoundedVec<u8, frame_support::traits::ConstU32<8192>> {
 		real_vk(TRANSFER_PUBLIC_INPUTS)
 	}
@@ -855,6 +864,49 @@ mod tests {
 		});
 	}
 
+	/// A circuit id past 255 must be refused, not truncated.
+	///
+	/// `expected_public_inputs` takes a `u8`, so `circuit_id.0 as u8` maps 257
+	/// onto 1: a key would be validated against TRANSFER's arity and stored under
+	/// an id that no lookup can reach. Root-gated, so this is an operator-error
+	/// amplifier rather than an attack, but it fails silently — which is the part
+	/// worth closing. `purge_circuit` already guards the same lookup this way.
+	#[test]
+	fn register_vk_rejects_circuit_id_that_would_alias() {
+		new_test_ext().execute_with(|| {
+			// 257 & 0xFF == 1 == CircuitId::TRANSFER.
+			let aliasing = CircuitId(257);
+			assert_eq!(aliasing.0 as u8, CircuitId::TRANSFER.0 as u8);
+
+			assert_noop!(
+				ZkVerifier::register_verification_key(
+					root().into(),
+					aliasing,
+					1,
+					real_vk(TRANSFER_PUBLIC_INPUTS)
+				),
+				Error::<Test>::InvalidVerificationKey
+			);
+		});
+	}
+
+	/// Ids inside `u8` but outside the known table keep working: they carry no
+	/// expected arity, so only "deserializes as a BN254 key" applies.
+	#[test]
+	fn register_vk_allows_unmapped_ids_within_u8() {
+		new_test_ext().execute_with(|| {
+			let unmapped = CircuitId(200);
+			assert!(orbinum_zk_verifier::expected_public_inputs(200).is_none());
+
+			assert_ok!(ZkVerifier::register_verification_key(
+				root().into(),
+				unmapped,
+				1,
+				real_vk(TRANSFER_PUBLIC_INPUTS)
+			));
+		});
+	}
+
 	#[test]
 	fn register_vk_stores_key_and_emits_event() {
 		new_test_ext().execute_with(|| {
@@ -1551,7 +1603,12 @@ mod tests {
 	#[test]
 	fn genesis_registers_vk_at_version_1_and_activates_it() {
 		let storage = pallet::GenesisConfig::<Test> {
-			verification_keys: vec![(CircuitId::TRANSFER, vec![0xCCu8; 300])],
+			// Genesis now applies the same arity check as registration, so a
+			// filler byte string no longer stands in for a key.
+			verification_keys: vec![(
+				CircuitId::TRANSFER,
+				real_vk(TRANSFER_PUBLIC_INPUTS).into_inner(),
+			)],
 			_phantom: Default::default(),
 		}
 		.build_storage()
@@ -1573,8 +1630,14 @@ mod tests {
 	fn genesis_multiple_circuits_are_all_registered() {
 		let storage = pallet::GenesisConfig::<Test> {
 			verification_keys: vec![
-				(CircuitId::TRANSFER, vec![0x11u8; 300]),
-				(CircuitId::UNSHIELD, vec![0x22u8; 300]),
+				(
+					CircuitId::TRANSFER,
+					real_vk(TRANSFER_PUBLIC_INPUTS).into_inner(),
+				),
+				(
+					CircuitId::UNSHIELD,
+					real_vk(UNSHIELD_PUBLIC_INPUTS).into_inner(),
+				),
 			],
 			_phantom: Default::default(),
 		}
@@ -1601,6 +1664,37 @@ mod tests {
 		});
 	}
 
+	/// Genesis must not accept a key that only passes the length check.
+	///
+	/// Before this, a well-sized but meaningless key was stored and the chain
+	/// found out when the first real proof failed to verify — at which point
+	/// nothing distinguishes a bad key from a bad proof. Failing at genesis
+	/// turns that into a chain that refuses to start.
+	#[test]
+	#[should_panic(expected = "Genesis VK must deserialize")]
+	fn genesis_rejects_a_key_that_only_passes_the_length_check() {
+		let _ = pallet::GenesisConfig::<Test> {
+			verification_keys: vec![(CircuitId::TRANSFER, vec![0xCCu8; 300])],
+			_phantom: Default::default(),
+		}
+		.build_storage();
+	}
+
+	/// A real key of the wrong arity is rejected too — deserializing is not
+	/// enough when the circuit declares how many inputs it takes.
+	#[test]
+	#[should_panic(expected = "Genesis VK must deserialize")]
+	fn genesis_rejects_a_valid_key_with_the_wrong_arity() {
+		let _ = pallet::GenesisConfig::<Test> {
+			verification_keys: vec![(
+				CircuitId::TRANSFER,
+				real_vk(TRANSFER_PUBLIC_INPUTS + 1).into_inner(),
+			)],
+			_phantom: Default::default(),
+		}
+		.build_storage();
+	}
+
 	// ── retire_version / unretire_version ─────────────────────────────────────
 
 	#[test]

primitives/zk-verifier/CHANGELOG.md

@@ -4,6 +4,53 @@ All notable changes to this crate are documented here.
 
 ---
 
+## [1.4.0] - 2026-08-07
+
+### Security
+
+- **Deserialization size guards moved inside the crate.** `to_ark_vk` and
+  `to_ark_proof` now reject oversized input before handing it to
+  `ark-serialize`.
+
+  A verifying key's `gamma_abc_g1` is a length-prefixed vector, and
+  `ark-serialize` calls `Vec::with_capacity` on that prefix **before reading a
+  single element**. A key declaring 2^40 points asks the allocator for ~48 GB on
+  nothing but submitted bytes: in Wasm that traps, on a native path it can take
+  the node down with it.
+
+  The two on-chain callers already bounded their argument at 8 KiB, so nothing
+  was reachable today. But the bound lived in the caller, not the function:
+  `to_ark_vk`, `num_public_inputs` and `prepare` are public API with no length
+  precondition, so any future caller — runtime API, offchain worker, an unsigned
+  path — inherited the reservation unguarded. New `MAX_VK_BYTES` (8 KiB, matching
+  the extrinsic bound) and `MAX_PROOF_BYTES` (1 KiB against a fixed 128-byte
+  compressed Groth16 proof). `prepare` and `num_public_inputs` route through
+  `to_ark_vk`, so they inherit the check.
+
+- **`MAX_PUBLIC_INPUTS` is now enforced.** The constant was declared and had zero
+  call sites outside its own definition: `to_field_elements` accepted a
+  `PublicInputs` of any length. The pallet bounds its extrinsic argument, but the
+  `ZkVerifierPort` path does not go through that extrinsic. Applied in
+  `to_field_elements`, the single point every input passes through. Every circuit
+  in use declares 7 inputs or fewer, so the limit cannot bite a real proof.
+
+### Fixed
+
+- `estimate_verification_cost` uses saturating arithmetic. The release profile
+  does not enable `overflow-checks`, so plain `*` and `+` would wrap silently and
+  report a cost far below the real one. Indicative only — on-chain weights come
+  from benchmarks — but a silently wrong number is worse than a large one.
+
+### Notes
+
+- Each guard was verified by removing it and confirming the test fails. Two of
+  the first drafts did **not**: they used filler bytes, which also fail to
+  deserialize, so the error arrived by another route and the test passed either
+  way. They now use a genuine BN254 key at arity 400 (over 8 KiB, and would
+  otherwise deserialize) and a real proof padded past the bound.
+
+---
+
 ## [1.3.0] - 2026-07-09
 
 ### Fixed

primitives/zk-verifier/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "orbinum-zk-verifier"
-version = "1.3.0"
+version = "1.4.0"
 authors = ["Orbinum Network <contact@orbinum.net>"]
 edition = "2021"
 publish = false