From 72aea4cef2101058cc9bf380774d5596f33c1f5c Mon Sep 17 00:00:00 2001 From: Eric Price Date: Tue, 30 Jun 2026 15:41:18 -0400 Subject: [PATCH 1/2] fix(channels-sv2): reject non-finite input in hash_rate_to_target MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit hash_rate_to_target guards only against zero/negative share_per_min and negative hashrate. It does NOT screen for non-finite values, and the intermediate `h * s as u128` cast saturates silently: `NaN as u128` is 0 and `f64::INFINITY as u128` is u128::MAX. So a NaN or infinite argument slips past every existing guard and yields a garbage target with no error. The +inf case is the dangerous one. A +inf hashrate casts to the maximum possible work, which collapses the target toward zero — the HARDEST difficulty the function can emit. So a single non-finite hashrate silently produces a maximally over-difficult target (the over-difficulty direction), from a converter whose contract is to reject bad input. NaN and 0.0 produce the easiest target instead, but the asymmetry is the point: one non-finite input drives difficulty to the ceiling. Reject non-finite hashrate AND share_per_min up front, with a dedicated HashRateToTargetError::NonFiniteInput variant. The check is ordered FIRST, ahead of the zero/negative checks: a NaN compares false to 0.0 and is sign-positive, and -inf is sign-negative, so without the leading finite screen they would fall through to DivisionByZero / NegativeInput / the cast. The ordering is the soundness property and is pinned by a test. Pure robustness fix — input validation only, no behavioral change to any caller's control path. 6 unit tests: each non-finite value on each operand, the load-bearing ordering (-inf and NaN screened as NonFiniteInput, not Negative/DivisionByZero), the +inf-yields-no-target headline, and that the pre-existing finite guards and valid conversions are unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) --- sv2/channels-sv2/src/target.rs | 108 +++++++++++++++++++++++++++++++++ 1 file changed, 108 insertions(+) diff --git a/sv2/channels-sv2/src/target.rs b/sv2/channels-sv2/src/target.rs index 08b8748c7f..e7773aa2bc 100644 --- a/sv2/channels-sv2/src/target.rs +++ b/sv2/channels-sv2/src/target.rs @@ -78,6 +78,21 @@ pub fn hash_rate_to_target( hashrate: f64, share_per_min: f64, ) -> Result { + // Reject non-finite input before any arithmetic. ORDER IS LOAD-BEARING: + // this check MUST precede the zero/negative checks below. A NaN compares + // false to everything (`NaN == 0.0` is false) and has its sign bit clear + // (`NaN.is_sign_negative()` is false), so it slips past every guard below + // and reaches the `as u128` cast, which saturates silently: `NaN as u128` + // is `0` and `f64::INFINITY as u128` is `u128::MAX`. A NaN or +inf hashrate + // would then yield a garbage target with no error — and `+inf` is the + // dangerous one: it casts to the maximum work, collapsing the target toward + // zero, i.e. the HARDEST difficulty (the over-difficulty / spiral + // direction). `-inf` is also caught here (it would otherwise return + // `NegativeInput`); keeping all non-finite rejection in one check ahead of + // the others is what makes the guard sound. + if !hashrate.is_finite() || !share_per_min.is_finite() { + return Err(HashRateToTargetError::NonFiniteInput); + } // checks that we are not dividing by zero if share_per_min == 0.0 { return Err(HashRateToTargetError::DivisionByZero); @@ -133,6 +148,11 @@ pub fn from_u128_to_u256(input: u128) -> U256Primitive { pub enum HashRateToTargetError { DivisionByZero, NegativeInput, + /// A `hashrate` or `share_per_min` argument was non-finite (`NaN` or + /// `±infinity`). These cast to nonsense `u128` work values (`NaN` → `0`, + /// `+inf` → `u128::MAX`) and would silently produce a garbage target, so + /// they are rejected before any conversion. + NonFiniteInput, } #[derive(Debug)] @@ -200,3 +220,91 @@ pub fn hash_rate_from_target(target: U256<'static>, share_per_min: f64) -> Resul // we multiply back by 100 so that it cancels with the same factor at the denominator Ok(result as f64) } + +#[cfg(test)] +mod tests { + use super::*; + + // A representative valid input still converts (regression guard: the + // non-finite screen must not reject ordinary finite values). + #[test] + fn finite_input_still_converts() { + assert!(hash_rate_to_target(1_000.0, 1.0).is_ok()); + // zero hashrate is finite and non-negative — still accepted, as before. + assert!(hash_rate_to_target(0.0, 1.0).is_ok()); + } + + // Each non-finite hashrate is rejected with the dedicated variant. + #[test] + fn non_finite_hashrate_is_rejected() { + for hashrate in [f64::NAN, f64::INFINITY, f64::NEG_INFINITY] { + assert!( + matches!( + hash_rate_to_target(hashrate, 1.0), + Err(HashRateToTargetError::NonFiniteInput) + ), + "hashrate {hashrate} should be NonFiniteInput", + ); + } + } + + // Each non-finite share_per_min is rejected — BOTH operands are screened, + // not just the hashrate. + #[test] + fn non_finite_share_per_min_is_rejected() { + for spm in [f64::NAN, f64::INFINITY, f64::NEG_INFINITY] { + assert!( + matches!( + hash_rate_to_target(1_000.0, spm), + Err(HashRateToTargetError::NonFiniteInput) + ), + "share_per_min {spm} should be NonFiniteInput", + ); + } + } + + // ORDER IS LOAD-BEARING. A `-inf` argument is BOTH non-finite AND + // sign-negative; if the negative check ran first it would return + // `NegativeInput`. Pinning `NonFiniteInput` here proves the finite screen + // precedes the negative screen — the property that keeps the guard sound. + #[test] + fn neg_infinity_is_non_finite_not_negative() { + assert!(matches!( + hash_rate_to_target(f64::NEG_INFINITY, 1.0), + Err(HashRateToTargetError::NonFiniteInput) + )); + // and a NaN share_per_min must not slip through to DivisionByZero/ + // NegativeInput (it compares false to 0.0 and is sign-positive). + assert!(matches!( + hash_rate_to_target(1_000.0, f64::NAN), + Err(HashRateToTargetError::NonFiniteInput) + )); + } + + // The headline case: a `+inf` hashrate used to cast to `u128::MAX` work and + // collapse the target toward zero — the HARDEST difficulty (the + // over-difficulty / spiral direction). It must now be rejected outright, + // never silently converted to that dangerous-direction target. + #[test] + fn positive_infinity_hashrate_does_not_yield_a_target() { + assert!(matches!( + hash_rate_to_target(f64::INFINITY, 1.0), + Err(HashRateToTargetError::NonFiniteInput) + )); + } + + // The pre-existing finite guards are unchanged: a genuinely negative finite + // hashrate is still NegativeInput, and zero share_per_min still + // DivisionByZero — the new screen narrows nothing that worked before. + #[test] + fn preexisting_finite_guards_unchanged() { + assert!(matches!( + hash_rate_to_target(-1.0, 1.0), + Err(HashRateToTargetError::NegativeInput) + )); + assert!(matches!( + hash_rate_to_target(1_000.0, 0.0), + Err(HashRateToTargetError::DivisionByZero) + )); + } +} From dd0761592dceaf0fc67b8257d837ae07b6eeb693 Mon Sep 17 00:00:00 2001 From: Eric Price Date: Tue, 28 Jul 2026 13:03:34 -0400 Subject: [PATCH 2/2] fix(channels-sv2): validate derived work, not just the operands Both operands can be finite while `hashrate * (60 / share_per_min)` is non-finite or too large for u128: `f64::from(f32::MAX)` is inside the domain of the f32 channel callers, and at 1 share/min it yields ~2.04e40, 60x above u128::MAX. `as u128` saturates rather than wrapping, so that became u128::MAX and then overflowed on `h_times_s + 1` -- a panic in debug, and a silent garbage target in release. Validate the product before the cast, with headroom for the `+ 1`, and return a dedicated `WorkOutOfRange` variant. Both new variants are enum_variant_added breaks against the published 7.0.0, so bump channels_sv2 to 8.0.0 and move the two dependents (stratum-core, stratum-translation) to ^8.0.0. Reported by @par1ram; version bump requested by @GitGab19. Co-Authored-By: Claude --- Cargo.lock | 2 +- stratum-core/Cargo.toml | 2 +- stratum-core/stratum-translation/Cargo.toml | 2 +- sv2/channels-sv2/Cargo.toml | 2 +- sv2/channels-sv2/src/target.rs | 83 +++++++++++++-------- 5 files changed, 55 insertions(+), 36 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a63b86abfe..e0f127c267 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -276,7 +276,7 @@ dependencies = [ [[package]] name = "channels_sv2" -version = "7.0.0" +version = "8.0.0" dependencies = [ "binary_sv2", "bitcoin", diff --git a/stratum-core/Cargo.toml b/stratum-core/Cargo.toml index 87f2a3193b..94caf3416c 100644 --- a/stratum-core/Cargo.toml +++ b/stratum-core/Cargo.toml @@ -20,7 +20,7 @@ framing_sv2 = { path = "../sv2/framing-sv2", version = "^7.0.0" } noise_sv2 = { path = "../sv2/noise-sv2", version = "^1.0.0" } parsers_sv2 = { path = "../sv2/parsers-sv2", version = "^0.5.0" } handlers_sv2 = { path = "../sv2/handlers-sv2", version = "^0.5.0" } -channels_sv2 = { path = "../sv2/channels-sv2", version = "^7.0.0" } +channels_sv2 = { path = "../sv2/channels-sv2", version = "^8.0.0" } common_messages_sv2 = { path = "../sv2/subprotocols/common-messages", version = "^8.0.0" } mining_sv2 = { path = "../sv2/subprotocols/mining", version = "^11.0.0" } template_distribution_sv2 = { path = "../sv2/subprotocols/template-distribution", version = "^6.0.0" } diff --git a/stratum-core/stratum-translation/Cargo.toml b/stratum-core/stratum-translation/Cargo.toml index dcc7c5add8..d0f1e3ae34 100644 --- a/stratum-core/stratum-translation/Cargo.toml +++ b/stratum-core/stratum-translation/Cargo.toml @@ -12,7 +12,7 @@ path = "src/lib.rs" [dependencies] binary_sv2 = { path = "../../sv2/binary-sv2", version = "^6.0.0" } mining_sv2 = { path = "../../sv2/subprotocols/mining", version = "^11.0.0" } -channels_sv2 = { path = "../../sv2/channels-sv2", version = "^7.0.0" } +channels_sv2 = { path = "../../sv2/channels-sv2", version = "^8.0.0" } v1 = { path = "../../sv1", package = "sv1_api", version = "^5.0.0" } tracing = { workspace = true } bitcoin = { workspace = true } diff --git a/sv2/channels-sv2/Cargo.toml b/sv2/channels-sv2/Cargo.toml index b3d2636ba5..47d8ed9342 100644 --- a/sv2/channels-sv2/Cargo.toml +++ b/sv2/channels-sv2/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "channels_sv2" -version = "7.0.0" +version = "8.0.0" authors = ["The Stratum V2 Developers"] edition = "2021" readme = "README.md" diff --git a/sv2/channels-sv2/src/target.rs b/sv2/channels-sv2/src/target.rs index e7773aa2bc..4a4cbc590c 100644 --- a/sv2/channels-sv2/src/target.rs +++ b/sv2/channels-sv2/src/target.rs @@ -78,18 +78,9 @@ pub fn hash_rate_to_target( hashrate: f64, share_per_min: f64, ) -> Result { - // Reject non-finite input before any arithmetic. ORDER IS LOAD-BEARING: - // this check MUST precede the zero/negative checks below. A NaN compares - // false to everything (`NaN == 0.0` is false) and has its sign bit clear - // (`NaN.is_sign_negative()` is false), so it slips past every guard below - // and reaches the `as u128` cast, which saturates silently: `NaN as u128` - // is `0` and `f64::INFINITY as u128` is `u128::MAX`. A NaN or +inf hashrate - // would then yield a garbage target with no error — and `+inf` is the - // dangerous one: it casts to the maximum work, collapsing the target toward - // zero, i.e. the HARDEST difficulty (the over-difficulty / spiral - // direction). `-inf` is also caught here (it would otherwise return - // `NegativeInput`); keeping all non-finite rejection in one check ahead of - // the others is what makes the guard sound. + // Must precede the zero/negative checks: a NaN compares false to everything + // and is sign-positive, so it would slip past them into the `as u128` cast, + // which saturates silently (`NaN` -> 0, `+inf` -> `u128::MAX`). if !hashrate.is_finite() || !share_per_min.is_finite() { return Err(HashRateToTargetError::NonFiniteInput); } @@ -109,6 +100,14 @@ pub fn hash_rate_to_target( let shares_occurrency_frequence = 60_f64 / share_per_min; let h_times_s = hashrate * shares_occurrency_frequence; + + // Finite operands can still yield a product that is non-finite or too large + // for `u128`: `f64::from(f32::MAX)` at 1 share/min gives ~2.04e40, 60x above + // `u128::MAX`. Bound it below `u128::MAX` so the `+ 1` below cannot overflow. + if !h_times_s.is_finite() || h_times_s >= u128::MAX as f64 { + return Err(HashRateToTargetError::WorkOutOfRange); + } + let h_times_s = h_times_s as u128; // We calculate the denominator: h*s+1 @@ -148,11 +147,11 @@ pub fn from_u128_to_u256(input: u128) -> U256Primitive { pub enum HashRateToTargetError { DivisionByZero, NegativeInput, - /// A `hashrate` or `share_per_min` argument was non-finite (`NaN` or - /// `±infinity`). These cast to nonsense `u128` work values (`NaN` → `0`, - /// `+inf` → `u128::MAX`) and would silently produce a garbage target, so - /// they are rejected before any conversion. + /// A `hashrate` or `share_per_min` argument was `NaN` or `±infinity`. NonFiniteInput, + /// The derived work `hashrate * (60 / share_per_min)` was non-finite or too + /// large for `u128`, even though both arguments were finite. + WorkOutOfRange, } #[derive(Debug)] @@ -225,16 +224,12 @@ pub fn hash_rate_from_target(target: U256<'static>, share_per_min: f64) -> Resul mod tests { use super::*; - // A representative valid input still converts (regression guard: the - // non-finite screen must not reject ordinary finite values). #[test] fn finite_input_still_converts() { assert!(hash_rate_to_target(1_000.0, 1.0).is_ok()); - // zero hashrate is finite and non-negative — still accepted, as before. assert!(hash_rate_to_target(0.0, 1.0).is_ok()); } - // Each non-finite hashrate is rejected with the dedicated variant. #[test] fn non_finite_hashrate_is_rejected() { for hashrate in [f64::NAN, f64::INFINITY, f64::NEG_INFINITY] { @@ -248,8 +243,6 @@ mod tests { } } - // Each non-finite share_per_min is rejected — BOTH operands are screened, - // not just the hashrate. #[test] fn non_finite_share_per_min_is_rejected() { for spm in [f64::NAN, f64::INFINITY, f64::NEG_INFINITY] { @@ -263,28 +256,22 @@ mod tests { } } - // ORDER IS LOAD-BEARING. A `-inf` argument is BOTH non-finite AND - // sign-negative; if the negative check ran first it would return - // `NegativeInput`. Pinning `NonFiniteInput` here proves the finite screen - // precedes the negative screen — the property that keeps the guard sound. + // Pins the check order: `-inf` is both non-finite and sign-negative, so a + // `NegativeInput` here would mean the negative check ran first. #[test] fn neg_infinity_is_non_finite_not_negative() { assert!(matches!( hash_rate_to_target(f64::NEG_INFINITY, 1.0), Err(HashRateToTargetError::NonFiniteInput) )); - // and a NaN share_per_min must not slip through to DivisionByZero/ - // NegativeInput (it compares false to 0.0 and is sign-positive). assert!(matches!( hash_rate_to_target(1_000.0, f64::NAN), Err(HashRateToTargetError::NonFiniteInput) )); } - // The headline case: a `+inf` hashrate used to cast to `u128::MAX` work and - // collapse the target toward zero — the HARDEST difficulty (the - // over-difficulty / spiral direction). It must now be rejected outright, - // never silently converted to that dangerous-direction target. + // `+inf` cast to `u128::MAX` work, collapsing the target toward zero (the + // hardest difficulty). Must be rejected, not silently converted. #[test] fn positive_infinity_hashrate_does_not_yield_a_target() { assert!(matches!( @@ -293,6 +280,38 @@ mod tests { )); } + // `f32::MAX` is in the domain of the `f32` channel callers; at 1 share/min the + // product is 60x above `u128::MAX`, which used to saturate and then overflow + // `h_times_s + 1`. + #[test] + fn work_exceeding_u128_is_rejected_not_saturated() { + assert!(matches!( + hash_rate_to_target(f64::from(f32::MAX), 1.0), + Err(HashRateToTargetError::WorkOutOfRange) + )); + } + + // Same gap via a non-finite product rather than a too-large one. + #[test] + fn non_finite_derived_work_is_rejected() { + let h_times_s = f64::MAX * (60.0 / 1e-300_f64); + assert!( + !h_times_s.is_finite(), + "precondition: product must overflow" + ); + assert!(matches!( + hash_rate_to_target(f64::MAX, 1e-300), + Err(HashRateToTargetError::WorkOutOfRange) + )); + } + + // Guards against over-rejection and pins the boundary. + #[test] + fn work_just_below_the_limit_still_converts() { + let just_under = (u128::MAX as f64) * 0.99; + assert!(hash_rate_to_target(just_under, 60.0).is_ok()); + } + // The pre-existing finite guards are unchanged: a genuinely negative finite // hashrate is still NegativeInput, and zero share_per_min still // DivisionByZero — the new screen narrows nothing that worked before.