Fix security and performance audit findings

Security:
- Validate Binance query params to prevent URL parameter injection
- Use saturating_abs() in risk checks to fix negative price bypass and i64::MIN panic
- Fail all risk checks when equity <= 0 (was silently passing)
- Guard CostModel u128→i64 cast with try_from

Performance:
- O(N) rolling metrics via running sum/sum_sq (was O(N*K))
- Eliminate 3 Vec allocations in RSI/MACD hot paths
- Compute CVaR tail mean on iterator (no intermediate Vec)

Author: ricardofrantz

broker/src/binance/client.rs

@@ -9,6 +9,20 @@ use super::auth;
 use super::types::{AccountInfo, BookTicker, OrderResponse};
 use crate::error::BrokerError;
 
+/// Validate that a parameter value is safe for URL query strings.
+///
+/// Rejects any value containing characters that could inject additional
+/// query parameters (e.g., `&`, `=`, `?`, `#`, space).
+fn validate_query_param(value: &str, name: &str) -> Result<(), BrokerError> {
+    if value.bytes().all(|b| b.is_ascii_alphanumeric() || b == b'_' || b == b'.' || b == b'-') {
+        Ok(())
+    } else {
+        Err(BrokerError::Order(format!(
+            "invalid characters in {name}: {value:?}"
+        )))
+    }
+}
+
 /// Blocking Binance REST client.
 pub struct BinanceClient {
     client: Client,
@@ -98,6 +112,17 @@ impl BinanceClient {
         price: Option<&str>,
         time_in_force: Option<&str>,
     ) -> Result<OrderResponse, BrokerError> {
+        validate_query_param(symbol, "symbol")?;
+        validate_query_param(side, "side")?;
+        validate_query_param(order_type, "order_type")?;
+        validate_query_param(quantity, "quantity")?;
+        if let Some(p) = price {
+            validate_query_param(p, "price")?;
+        }
+        if let Some(tif) = time_in_force {
+            validate_query_param(tif, "timeInForce")?;
+        }
+
         let timestamp = current_timestamp_ms();
         let mut query = format!(
             "symbol={symbol}&side={side}&type={order_type}&quantity={quantity}&timestamp={timestamp}"
@@ -137,6 +162,7 @@ impl BinanceClient {
 
     /// Get order status (GET /api/v3/order).
     pub fn order_status(&self, symbol: &str, order_id: u64) -> Result<OrderResponse, BrokerError> {
+        validate_query_param(symbol, "symbol")?;
         let timestamp = current_timestamp_ms();
         let query = format!("symbol={symbol}&orderId={order_id}&timestamp={timestamp}");
         let signature = auth::sign(&query, &self.secret_key);
@@ -166,6 +192,7 @@ impl BinanceClient {
 
     /// Cancel an order (DELETE /api/v3/order).
     pub fn cancel_order(&self, symbol: &str, order_id: u64) -> Result<(), BrokerError> {
+        validate_query_param(symbol, "symbol")?;
         let timestamp = current_timestamp_ms();
         let query = format!("symbol={symbol}&orderId={order_id}&timestamp={timestamp}");
         let signature = auth::sign(&query, &self.secret_key);
@@ -194,6 +221,7 @@ impl BinanceClient {
 
     /// Get book ticker (best bid/ask) for a symbol (GET /api/v3/ticker/bookTicker).
     pub fn book_ticker(&self, symbol: &str) -> Result<BookTicker, BrokerError> {
+        validate_query_param(symbol, "symbol")?;
         let url = format!("{}/api/v3/ticker/bookTicker?symbol={symbol}", self.base_url);
 
         let resp = self

risk/src/checks.rs

@@ -68,14 +68,14 @@ pub fn check_batch(
     let gross_exposure: i64 = post_qty
         .iter()
         .map(|(sym, qty)| {
-            let price = price_map.get(sym).copied().unwrap_or(0);
-            qty.abs() * price
+            let price = price_map.get(sym).copied().unwrap_or(0).saturating_abs();
+            qty.saturating_abs().saturating_mul(price)
         })
-        .sum();
+        .fold(0_i64, |acc, v| acc.saturating_add(v));
     let leverage = if equity > 0 {
         gross_exposure as f64 / equity as f64
     } else {
-        0.0
+        f64::INFINITY
     };
     let lev_status = if leverage > config.max_leverage {
         RiskStatus::Fail
@@ -102,14 +102,14 @@ pub fn check_batch(
         .iter()
         .filter(|(_, qty)| **qty < 0)
         .map(|(sym, qty)| {
-            let price = price_map.get(sym).copied().unwrap_or(0);
-            qty.abs() * price
+            let price = price_map.get(sym).copied().unwrap_or(0).saturating_abs();
+            qty.saturating_abs().saturating_mul(price)
         })
-        .sum();
+        .fold(0_i64, |acc, v| acc.saturating_add(v));
     let short_pct = if equity > 0 {
         short_exposure as f64 / equity as f64
     } else {
-        0.0
+        f64::INFINITY
     };
 
     let has_shorts = orders
@@ -149,7 +149,7 @@ pub fn check_batch(
     let max_cents = (config.max_trade_usd * 100.0) as i64;
     for &(sym, _side, qty, price) in orders {
         let qty_i64 = i64::try_from(qty).unwrap_or(i64::MAX);
-        let notional = qty_i64.saturating_mul(price);
+        let notional = qty_i64.saturating_mul(price.saturating_abs());
         if notional > max_cents {
             checks.push(RiskCheck {
                 name: "Max trade size",

src/indicators.rs

@@ -62,6 +62,7 @@ fn sma(values: &[f64], period: usize) -> Vec<f64> {
 
 /// Population standard deviation over a rolling window.
 ///
+/// Uses O(N) running sum/sum-of-squares instead of O(N*K) re-summation.
 /// Returns NaN for the lookback period.
 fn rolling_std_pop(values: &[f64], period: usize) -> Vec<f64> {
     let n = values.len();
@@ -70,11 +71,24 @@ fn rolling_std_pop(values: &[f64], period: usize) -> Vec<f64> {
         return out;
     }
 
-    for i in (period - 1)..n {
-        let window = &values[i + 1 - period..=i];
-        let mean = window.iter().sum::<f64>() / period as f64;
-        let var = window.iter().map(|&v| (v - mean).powi(2)).sum::<f64>() / period as f64;
-        out[i] = var.sqrt();
+    let k = period as f64;
+
+    // Seed: first window
+    let mut sum: f64 = values[..period].iter().sum();
+    let mut sum_sq: f64 = values[..period].iter().map(|v| v * v).sum();
+
+    let mean = sum / k;
+    out[period - 1] = (sum_sq / k - mean * mean).max(0.0).sqrt();
+
+    // Slide window: add new, remove old
+    for i in period..n {
+        let old = values[i - period];
+        let new = values[i];
+        sum += new - old;
+        sum_sq += new * new - old * old;
+
+        let mean = sum / k;
+        out[i] = (sum_sq / k - mean * mean).max(0.0).sqrt();
     }
     out
 }
@@ -114,21 +128,19 @@ pub fn rsi(close: &[f64], period: usize) -> Vec<f64> {
         return out;
     }
 
-    // Compute gains and losses
-    let mut gains = vec![0.0_f64; n];
-    let mut losses = vec![0.0_f64; n];
-    for i in 1..n {
+    // Seed with simple average over first `period` changes (indices 1..=period)
+    let mut avg_gain = 0.0_f64;
+    let mut avg_loss = 0.0_f64;
+    for i in 1..=period {
         let diff = close[i] - close[i - 1];
         if diff > 0.0 {
-            gains[i] = diff;
+            avg_gain += diff;
         } else {
-            losses[i] = -diff;
+            avg_loss -= diff;
         }
     }
-
-    // Seed with simple average over first `period` changes (indices 1..=period)
-    let mut avg_gain: f64 = gains[1..=period].iter().sum::<f64>() / period as f64;
-    let mut avg_loss: f64 = losses[1..=period].iter().sum::<f64>() / period as f64;
+    avg_gain /= period as f64;
+    avg_loss /= period as f64;
 
     // First RSI value
     out[period] = if avg_gain == 0.0 && avg_loss == 0.0 {
@@ -142,8 +154,11 @@ pub fn rsi(close: &[f64], period: usize) -> Vec<f64> {
 
     // Subsequent values with Wilder's smoothing
     for i in (period + 1)..n {
-        avg_gain = (avg_gain * (period as f64 - 1.0) + gains[i]) / period as f64;
-        avg_loss = (avg_loss * (period as f64 - 1.0) + losses[i]) / period as f64;
+        let diff = close[i] - close[i - 1];
+        let gain = if diff > 0.0 { diff } else { 0.0 };
+        let loss = if diff < 0.0 { -diff } else { 0.0 };
+        avg_gain = (avg_gain * (period as f64 - 1.0) + gain) / period as f64;
+        avg_loss = (avg_loss * (period as f64 - 1.0) + loss) / period as f64;
 
         out[i] = if avg_gain == 0.0 && avg_loss == 0.0 {
             0.0
@@ -210,9 +225,8 @@ pub fn macd(
         }
     }
 
-    // Signal line = EMA of valid MACD values
-    let valid_macd: Vec<f64> = macd_line[first_valid..].to_vec();
-    let signal_raw = ema(&valid_macd, signal_period);
+    // Signal line = EMA of valid MACD values (pass slice directly — no copy)
+    let signal_raw = ema(&macd_line[first_valid..], signal_period);
 
     let mut signal_line = vec![f64::NAN; n];
     for (j, &val) in signal_raw.iter().enumerate() {

src/portfolio/cost_model.rs

@@ -41,7 +41,8 @@ impl CostModel {
         let notional = notional.unsigned_abs() as u128;
         let total_bps = self.commission_bps as u128 + self.slippage_bps as u128;
         // notional * bps / 10_000 — use u128 to prevent overflow
-        let bps_cost = (notional * total_bps / 10_000) as i64;
+        let raw = notional * total_bps / 10_000;
+        let bps_cost = i64::try_from(raw).unwrap_or(i64::MAX);
         bps_cost.max(self.min_trade_fee)
     }
 }

src/portfolio/metrics.rs

@@ -255,15 +255,18 @@ fn compute_cvar(returns: &[f64], alpha: f64) -> f64 {
     let z = norm_ppf(alpha);
     let var_threshold = mu + sigma * z;
 
-    // CVaR: mean of returns strictly below VaR
-    let tail: Vec<f64> = returns.iter().copied().filter(|&r| r < var_threshold).collect();
-    if tail.is_empty() {
+    // CVaR: mean of returns strictly below VaR (computed on iterator — no allocation)
+    let (tail_sum, tail_count) = returns
+        .iter()
+        .filter(|&&r| r < var_threshold)
+        .fold((0.0_f64, 0_usize), |(sum, cnt), &r| (sum + r, cnt + 1));
+    if tail_count == 0 {
         return *returns
             .iter()
             .min_by(|a, b| a.partial_cmp(b).unwrap_or(std::cmp::Ordering::Equal))
             .unwrap_or(&0.0);
     }
-    tail.iter().sum::<f64>() / tail.len() as f64
+    tail_sum / tail_count as f64
 }
 
 /// Inverse of the standard normal CDF (probit function).
@@ -351,17 +354,28 @@ pub fn rolling_sharpe(returns: &[f64], window: usize, periods_per_year: usize) -
     }
 
     let ppy = periods_per_year as f64;
+    let k = window as f64;
 
-    for i in (window - 1)..n {
-        let w = &returns[i + 1 - window..=i];
-        let mean = w.iter().sum::<f64>() / window as f64;
-        let var = w.iter().map(|&r| (r - mean).powi(2)).sum::<f64>() / (window - 1) as f64;
-        let std = var.sqrt();
-        out[i] = if std > 0.0 {
-            mean * ppy.sqrt() / std
-        } else {
-            0.0
-        };
+    // Seed first window
+    let mut sum: f64 = returns[..window].iter().sum();
+    let mut sum_sq: f64 = returns[..window].iter().map(|r| r * r).sum();
+
+    let mean = sum / k;
+    let var = (sum_sq - sum * sum / k) / (k - 1.0);
+    let std = var.max(0.0).sqrt();
+    out[window - 1] = if std > 0.0 { mean * ppy.sqrt() / std } else { 0.0 };
+
+    // Slide window
+    for i in window..n {
+        let old = returns[i - window];
+        let new = returns[i];
+        sum += new - old;
+        sum_sq += new * new - old * old;
+
+        let mean = sum / k;
+        let var = (sum_sq - sum * sum / k) / (k - 1.0);
+        let std = var.max(0.0).sqrt();
+        out[i] = if std > 0.0 { mean * ppy.sqrt() / std } else { 0.0 };
     }
 
     out
@@ -384,12 +398,24 @@ pub fn rolling_volatility(returns: &[f64], window: usize, periods_per_year: usiz
     }
 
     let ppy = periods_per_year as f64;
+    let k = window as f64;
+
+    // Seed first window
+    let mut sum: f64 = returns[..window].iter().sum();
+    let mut sum_sq: f64 = returns[..window].iter().map(|r| r * r).sum();
+
+    let var = (sum_sq - sum * sum / k) / (k - 1.0);
+    out[window - 1] = var.max(0.0).sqrt() * ppy.sqrt();
+
+    // Slide window
+    for i in window..n {
+        let old = returns[i - window];
+        let new = returns[i];
+        sum += new - old;
+        sum_sq += new * new - old * old;
 
-    for i in (window - 1)..n {
-        let w = &returns[i + 1 - window..=i];
-        let mean = w.iter().sum::<f64>() / window as f64;
-        let var = w.iter().map(|&r| (r - mean).powi(2)).sum::<f64>() / (window - 1) as f64;
-        out[i] = var.sqrt() * ppy.sqrt();
+        let var = (sum_sq - sum * sum / k) / (k - 1.0);
+        out[i] = var.max(0.0).sqrt() * ppy.sqrt();
     }
 
     out