From f7982f04157883111cdc594d4dd1a123a5d113c7 Mon Sep 17 00:00:00 2001 From: Parfii-bot Date: Thu, 23 Apr 2026 00:49:49 +0800 Subject: [PATCH] =?UTF-8?q?fix(substrate):=20E2=20=E2=80=94=20kei-forge=20?= =?UTF-8?q?security=20hardening=20(DNS=20rebind=20+=20CSRF=20+=20injection?= =?UTF-8?q?)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three HIGH security findings resolved in _primitives/_rust/kei-forge/: - F-1: DNS rebinding — require_local_host middleware returns 421 on non-localhost Host headers - F-2: CSRF via urlencoded — require_json_content_type middleware returns 415 on non-JSON; form HTML now POSTs JSON via fetch() - crit#1/SA F-7: description sed injection — whitelist validator rejects newline/CR/tab/NUL/backtick/$/length>200, blocks the shell-script attack at the Rust layer - crit#11: missing security headers — CSP, X-Frame-Options DENY, X-Content-Type-Options nosniff, Referrer-Policy no-referrer on GET / Zero new deps (axum 0.7 middleware::from_fn + HeaderMap native). Constructor Pattern compliant — 6 Cube files, largest 231 LOC including tests. Tests: 29/29 (was 12/12; +17 new). Includes 4 adversarial integration tests for each defence layer. Co-Authored-By: Claude Opus 4.7 (1M context) --- _primitives/_rust/kei-forge/src/form.rs | 92 ++++++++++ _primitives/_rust/kei-forge/src/headers.rs | 62 +++++++ _primitives/_rust/kei-forge/src/html.rs | 71 ++++++++ _primitives/_rust/kei-forge/src/lib.rs | 12 +- _primitives/_rust/kei-forge/src/middleware.rs | 117 +++++++++++++ _primitives/_rust/kei-forge/src/server.rs | 75 +++----- _primitives/_rust/kei-forge/tests/smoke.rs | 165 ++++++++++++++---- 7 files changed, 504 insertions(+), 90 deletions(-) create mode 100644 _primitives/_rust/kei-forge/src/headers.rs create mode 100644 _primitives/_rust/kei-forge/src/html.rs create mode 100644 _primitives/_rust/kei-forge/src/middleware.rs diff --git a/_primitives/_rust/kei-forge/src/form.rs b/_primitives/_rust/kei-forge/src/form.rs index c062fd3..fbe574e 100644 --- a/_primitives/_rust/kei-forge/src/form.rs +++ b/_primitives/_rust/kei-forge/src/form.rs @@ -24,9 +24,42 @@ pub fn validate(req: &ForgeRequest) -> Result<(), String> { validate_crate_name(&req.crate_name)?; validate_verb(&req.verb)?; validate_kind(&req.kind)?; + validate_description(&req.description)?; Ok(()) } +/// Description whitelist — ASCII printable only. +/// +/// Hardening against shell-substitution in `scripts/new-atom.sh`: an +/// attacker-controlled newline, backtick, or `$` could smuggle a +/// secondary `sed` expression into the template-substitution step and +/// poison generated Rust source. Blocking these at the Rust layer +/// prevents the shell from ever seeing a hostile byte. +fn validate_description(d: &str) -> Result<(), String> { + if d.len() > MAX_DESCRIPTION_LEN { + return Err(format!( + "description must be ≤{MAX_DESCRIPTION_LEN} chars (got {})", + d.len() + )); + } + for (i, b) in d.bytes().enumerate() { + if !(0x20..=0x7E).contains(&b) { + return Err(format!( + "description contains non-printable byte 0x{b:02X} at offset {i}" + )); + } + if matches!(b, b'`' | b'$') { + return Err(format!( + "description contains forbidden character '{}' at offset {i}", + b as char + )); + } + } + Ok(()) +} + +const MAX_DESCRIPTION_LEN: usize = 200; + fn validate_crate_name(name: &str) -> Result<(), String> { if name.is_empty() { return Err("crate must not be empty".to_string()); @@ -136,4 +169,63 @@ mod tests { }; assert!(validate(&req).is_err()); } + + fn req_with_desc(d: &str) -> ForgeRequest { + ForgeRequest { + crate_name: "kei-task".into(), + verb: "noop".into(), + kind: "command".into(), + description: d.into(), + } + } + + #[test] + fn description_rejects_newline() { + let err = validate(&req_with_desc("foo\nbar")).unwrap_err(); + assert!(err.contains("0x0A"), "expected newline byte report: {err}"); + } + + #[test] + fn description_rejects_carriage_return() { + assert!(validate(&req_with_desc("foo\rbar")).is_err()); + } + + #[test] + fn description_rejects_tab() { + assert!(validate(&req_with_desc("foo\tbar")).is_err()); + } + + #[test] + fn description_rejects_nul() { + assert!(validate(&req_with_desc("foo\0bar")).is_err()); + } + + #[test] + fn description_rejects_backtick() { + let err = validate(&req_with_desc("foo`id`bar")).unwrap_err(); + assert!(err.contains('`'), "expected backtick in error: {err}"); + } + + #[test] + fn description_rejects_dollar_sign() { + let err = validate(&req_with_desc("foo$(id)bar")).unwrap_err(); + assert!(err.contains('$'), "expected dollar in error: {err}"); + } + + #[test] + fn description_rejects_over_length() { + let long = "a".repeat(201); + assert!(validate(&req_with_desc(&long)).is_err()); + } + + #[test] + fn description_accepts_minimal() { + assert!(validate(&req_with_desc("ok")).is_ok()); + } + + #[test] + fn description_accepts_at_length_cap() { + let exact = "a".repeat(200); + assert!(validate(&req_with_desc(&exact)).is_ok()); + } } diff --git a/_primitives/_rust/kei-forge/src/headers.rs b/_primitives/_rust/kei-forge/src/headers.rs new file mode 100644 index 0000000..af2bafc --- /dev/null +++ b/_primitives/_rust/kei-forge/src/headers.rs @@ -0,0 +1,62 @@ +//! Security headers applied to the GET / HTML response. +//! +//! Defence-in-depth layer on top of the Host allow-list and JSON +//! content-type enforcement: these directives limit the blast radius of +//! any reflected-XSS / iframe-embedding attempt against the wizard UI. +//! +//! - `Content-Security-Policy` — inline-script/style only from self, no +//! external origins, `form-action 'self'` blocks cross-origin form +//! posts even if the SOP layer is bypassed. +//! - `X-Content-Type-Options: nosniff` — browsers MUST NOT sniff MIME. +//! - `X-Frame-Options: DENY` — cannot be iframe-embedded (clickjacking). +//! - `Referrer-Policy: no-referrer` — don't leak the wizard URL. + +use axum::http::{header, HeaderMap, HeaderValue}; + +/// Populate `headers` with the four security headers. Used by the GET / +/// handler to decorate its HTML response. +pub fn apply_security_headers(headers: &mut HeaderMap) { + headers.insert( + header::CONTENT_SECURITY_POLICY, + HeaderValue::from_static( + "default-src 'self'; script-src 'self' 'unsafe-inline'; \ + style-src 'self' 'unsafe-inline'; form-action 'self'; \ + frame-ancestors 'none'", + ), + ); + headers.insert( + header::X_CONTENT_TYPE_OPTIONS, + HeaderValue::from_static("nosniff"), + ); + headers.insert( + header::X_FRAME_OPTIONS, + HeaderValue::from_static("DENY"), + ); + headers.insert( + header::REFERRER_POLICY, + HeaderValue::from_static("no-referrer"), + ); +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn adds_all_four_headers() { + let mut h = HeaderMap::new(); + apply_security_headers(&mut h); + assert!(h.contains_key(header::CONTENT_SECURITY_POLICY)); + assert!(h.contains_key(header::X_CONTENT_TYPE_OPTIONS)); + assert!(h.contains_key(header::X_FRAME_OPTIONS)); + assert!(h.contains_key(header::REFERRER_POLICY)); + } + + #[test] + fn csp_forbids_cross_origin_forms() { + let mut h = HeaderMap::new(); + apply_security_headers(&mut h); + let csp = h.get(header::CONTENT_SECURITY_POLICY).unwrap(); + assert!(csp.to_str().unwrap().contains("form-action 'self'")); + } +} diff --git a/_primitives/_rust/kei-forge/src/html.rs b/_primitives/_rust/kei-forge/src/html.rs new file mode 100644 index 0000000..9f99f25 --- /dev/null +++ b/_primitives/_rust/kei-forge/src/html.rs @@ -0,0 +1,71 @@ +//! Static HTML for the wizard form. +//! +//! The form no longer POSTs `application/x-www-form-urlencoded` directly +//! — that body-type is SOP-safe (no CORS preflight) which allowed any +//! web page to CSRF POST the handler. Instead, a small inline ` + + +"#; diff --git a/_primitives/_rust/kei-forge/src/lib.rs b/_primitives/_rust/kei-forge/src/lib.rs index fcae8f1..2ddf2db 100644 --- a/_primitives/_rust/kei-forge/src/lib.rs +++ b/_primitives/_rust/kei-forge/src/lib.rs @@ -2,9 +2,12 @@ //! SUBSTRATE-SCHEMA.md contract. //! //! Architecture (Constructor Pattern, one responsibility per file): -//! - [`server`] — axum router + HTML form handler -//! - [`form`] — request deserialization + validation -//! - [`generate`] — invoke scripts/new-atom.sh, parse output +//! - [`server`] — axum router + handlers +//! - [`middleware`] — DNS-rebinding + CSRF defences +//! - [`headers`] — CSP / nosniff / frame-deny / referrer headers +//! - [`html`] — static HTML form (JSON-over-fetch) +//! - [`form`] — request deserialization + validation +//! - [`generate`] — invoke scripts/new-atom.sh, parse output //! //! Public entry point is [`server::app`], which returns the fully-wired //! `axum::Router` ready to be served by any bind target (production = @@ -12,4 +15,7 @@ pub mod form; pub mod generate; +pub mod headers; +pub mod html; +pub mod middleware; pub mod server; diff --git a/_primitives/_rust/kei-forge/src/middleware.rs b/_primitives/_rust/kei-forge/src/middleware.rs new file mode 100644 index 0000000..60d047a --- /dev/null +++ b/_primitives/_rust/kei-forge/src/middleware.rs @@ -0,0 +1,117 @@ +//! HTTP middleware — defence against cross-origin / DNS-rebinding attacks. +//! +//! Two layers: +//! - [`require_local_host`] — rejects requests whose `Host:` header is not +//! exactly `localhost:8747` or `127.0.0.1:8747`. Blocks DNS-rebinding +//! (attacker points `a.evil.com` → 127.0.0.1 while browser still trusts +//! the evil.com origin for Same-Origin-Policy purposes). +//! - [`require_json_content_type`] — rejects `POST /forge` unless body is +//! `application/json`. Blocks CSRF via `
` submissions: urlencoded +//! POSTs are SOP-safe (no preflight), but JSON bodies trigger CORS +//! preflight so SOP engages. +//! +//! Both are advisory: they compose via `axum::middleware::from_fn` and +//! never touch application state. + +use axum::{ + extract::Request, + http::{header, Method, StatusCode}, + middleware::Next, + response::Response, +}; + +const ALLOWED_HOSTS: &[&str] = &["localhost:8747", "127.0.0.1:8747"]; + +/// Reject requests whose `Host:` is not an exact allow-list match. +/// +/// Returns 421 Misdirected Request on mismatch (RFC 7540 §9.1.2). +pub async fn require_local_host(req: Request, next: Next) -> Result { + let host = req + .headers() + .get(header::HOST) + .and_then(|v| v.to_str().ok()) + .unwrap_or(""); + if ALLOWED_HOSTS.iter().any(|&h| h == host) { + Ok(next.run(req).await) + } else { + Err(StatusCode::MISDIRECTED_REQUEST) + } +} + +/// Reject POSTs whose `Content-Type` is not `application/json`. +/// +/// GET and other methods pass through unchanged. Returns 415 Unsupported +/// Media Type on mismatch. +pub async fn require_json_content_type( + req: Request, + next: Next, +) -> Result { + if req.method() != Method::POST { + return Ok(next.run(req).await); + } + let ct = req + .headers() + .get(header::CONTENT_TYPE) + .and_then(|v| v.to_str().ok()) + .unwrap_or(""); + // Match only the media type, ignoring optional `; charset=…`. + let base = ct.split(';').next().unwrap_or("").trim(); + if base.eq_ignore_ascii_case("application/json") { + Ok(next.run(req).await) + } else { + Err(StatusCode::UNSUPPORTED_MEDIA_TYPE) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use axum::body::Body; + use axum::http::Request as HttpRequest; + use axum::middleware::from_fn; + use axum::routing::{get, post}; + use axum::Router; + use tower::ServiceExt; + + fn test_app() -> Router { + Router::new() + .route("/", get(|| async { "ok" })) + .route("/forge", post(|| async { "ok" })) + .layer(from_fn(require_json_content_type)) + .layer(from_fn(require_local_host)) + } + + #[tokio::test] + async fn blocks_evil_host() { + let app = test_app(); + let resp = app + .oneshot( + HttpRequest::builder() + .uri("/") + .header("host", "evil.com") + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::MISDIRECTED_REQUEST); + } + + #[tokio::test] + async fn blocks_urlencoded_post() { + let app = test_app(); + let resp = app + .oneshot( + HttpRequest::builder() + .method("POST") + .uri("/forge") + .header("host", "127.0.0.1:8747") + .header("content-type", "application/x-www-form-urlencoded") + .body(Body::from("x=1")) + .unwrap(), + ) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::UNSUPPORTED_MEDIA_TYPE); + } +} diff --git a/_primitives/_rust/kei-forge/src/server.rs b/_primitives/_rust/kei-forge/src/server.rs index 72f9f3e..c2a355e 100644 --- a/_primitives/_rust/kei-forge/src/server.rs +++ b/_primitives/_rust/kei-forge/src/server.rs @@ -3,30 +3,49 @@ //! Intentionally stateless: no `AppState`, no handles, no async init. //! Every request is self-contained. This lets tests spin up `app()` in //! an ephemeral Tokio runtime without setup teardown overhead. +//! +//! Security layers applied as middleware (see `crate::middleware`): +//! 1. `require_local_host` — reject non-127.0.0.1 Host headers (blocks +//! DNS rebinding). +//! 2. `require_json_content_type` — reject urlencoded POSTs (blocks +//! ``-based CSRF). +//! +//! GET / responses additionally carry CSP + nosniff + frame-deny headers. use axum::{ - extract::Form, - http::StatusCode, - response::{Html, IntoResponse}, + http::{HeaderMap, StatusCode}, + middleware::from_fn, + response::IntoResponse, routing::{get, post}, Json, Router, }; use crate::form::{validate, ForgeRequest}; use crate::generate::{forge, ForgeResult}; +use crate::headers::apply_security_headers; +use crate::html::FORM_HTML; +use crate::middleware::{require_json_content_type, require_local_host}; /// Build the router. Called by `main.rs` and by tests. pub fn app() -> Router { Router::new() .route("/", get(render_form)) .route("/forge", post(handle_forge)) + .layer(from_fn(require_json_content_type)) + .layer(from_fn(require_local_host)) } -async fn render_form() -> Html<&'static str> { - Html(FORM_HTML) +async fn render_form() -> impl IntoResponse { + let mut headers = HeaderMap::new(); + apply_security_headers(&mut headers); + headers.insert( + axum::http::header::CONTENT_TYPE, + axum::http::HeaderValue::from_static("text/html; charset=utf-8"), + ); + (StatusCode::OK, headers, FORM_HTML) } -async fn handle_forge(Form(req): Form) -> impl IntoResponse { +async fn handle_forge(Json(req): Json) -> impl IntoResponse { if let Err(e) = validate(&req) { return (StatusCode::BAD_REQUEST, Json(ForgeResult::fail(e))); } @@ -38,47 +57,3 @@ async fn handle_forge(Form(req): Form) -> impl IntoResponse { }; (status, Json(result)) } - -/// Minimal HTML — 5 inputs, no JS, no CSS framework. The locked schema -/// allows only 4 atom kinds, hard-coded as a ` - -

-

- -

-

- -

-

- -

-

- - - -"#; diff --git a/_primitives/_rust/kei-forge/tests/smoke.rs b/_primitives/_rust/kei-forge/tests/smoke.rs index d43f2db..b322a73 100644 --- a/_primitives/_rust/kei-forge/tests/smoke.rs +++ b/_primitives/_rust/kei-forge/tests/smoke.rs @@ -9,24 +9,36 @@ use axum::{ body::{to_bytes, Body}, - http::{Request, StatusCode}, + http::{header, Request, StatusCode}, }; use kei_forge::server; use serde_json::Value; use tower::ServiceExt; +const LOCAL_HOST: &str = "127.0.0.1:8747"; + +fn get(uri: &str) -> Request { + Request::builder() + .uri(uri) + .header("host", LOCAL_HOST) + .body(Body::empty()) + .unwrap() +} + +fn post_json(uri: &str, body: &str) -> Request { + Request::builder() + .method("POST") + .uri(uri) + .header("host", LOCAL_HOST) + .header("content-type", "application/json") + .body(Body::from(body.to_string())) + .unwrap() +} + #[tokio::test] async fn get_root_serves_form() { let app = server::app(); - let resp = app - .oneshot( - Request::builder() - .uri("/") - .body(Body::empty()) - .unwrap(), - ) - .await - .unwrap(); + let resp = app.oneshot(get("/")).await.unwrap(); assert_eq!(resp.status(), StatusCode::OK); let body = to_bytes(resp.into_body(), 64 * 1024).await.unwrap(); @@ -40,19 +52,9 @@ async fn get_root_serves_form() { #[tokio::test] async fn post_forge_returns_json_shape() { let app = server::app(); - let body = "crate=kei-task&verb=add-dependency&kind=command&description=test+desc"; + let body = r#"{"crate":"kei-task","verb":"add-dependency","kind":"command","description":"test desc"}"#; - let resp = app - .oneshot( - Request::builder() - .method("POST") - .uri("/forge") - .header("content-type", "application/x-www-form-urlencoded") - .body(Body::from(body)) - .unwrap(), - ) - .await - .unwrap(); + let resp = app.oneshot(post_json("/forge", body)).await.unwrap(); let status = resp.status(); let bytes = to_bytes(resp.into_body(), 64 * 1024).await.unwrap(); @@ -62,9 +64,6 @@ async fn post_forge_returns_json_shape() { assert!(json.get("files").is_some(), "missing files field"); assert!(json.get("errors").is_some(), "missing errors field"); - // With --features mock-generate we return success; without, the shell-out - // may succeed or fail depending on whether scripts/new-atom.sh lives in - // the worktree. Either way the schema must be correct. assert!( status == StatusCode::OK || status == StatusCode::UNPROCESSABLE_ENTITY @@ -76,19 +75,9 @@ async fn post_forge_returns_json_shape() { #[tokio::test] async fn post_forge_rejects_bad_kind() { let app = server::app(); - let body = "crate=kei-task&verb=x&kind=saga&description=y"; + let body = r#"{"crate":"kei-task","verb":"x","kind":"saga","description":"y"}"#; - let resp = app - .oneshot( - Request::builder() - .method("POST") - .uri("/forge") - .header("content-type", "application/x-www-form-urlencoded") - .body(Body::from(body)) - .unwrap(), - ) - .await - .unwrap(); + let resp = app.oneshot(post_json("/forge", body)).await.unwrap(); assert_eq!(resp.status(), StatusCode::BAD_REQUEST); let bytes = to_bytes(resp.into_body(), 64 * 1024).await.unwrap(); @@ -97,3 +86,105 @@ async fn post_forge_rejects_bad_kind() { let errs = json["errors"].as_array().unwrap(); assert!(!errs.is_empty()); } + +// --------------------------------------------------------------------- +// Security hardening — the four new tests required by the fix contract. +// --------------------------------------------------------------------- + +/// FIX A (DNS rebinding): a POST whose `Host:` header names an attacker +/// domain — even when the underlying socket is 127.0.0.1 — MUST be +/// rejected before the handler sees it. +#[tokio::test] +async fn post_with_evil_host_is_rejected() { + let app = server::app(); + let resp = app + .oneshot( + Request::builder() + .method("POST") + .uri("/forge") + .header("host", "evil.com") + .header("content-type", "application/json") + .body(Body::from( + r#"{"crate":"kei-task","verb":"x","kind":"command","description":"y"}"#, + )) + .unwrap(), + ) + .await + .unwrap(); + assert_ne!(resp.status(), StatusCode::OK); + assert!( + resp.status() == StatusCode::MISDIRECTED_REQUEST + || resp.status() == StatusCode::FORBIDDEN, + "expected 403 or 421, got {}", + resp.status() + ); +} + +/// FIX B (CSRF): `application/x-www-form-urlencoded` is SOP-safe, so a +/// malicious `
` on any site could POST to us without preflight. +/// Must be rejected with 415 Unsupported Media Type. +#[tokio::test] +async fn post_urlencoded_is_rejected() { + let app = server::app(); + let resp = app + .oneshot( + Request::builder() + .method("POST") + .uri("/forge") + .header("host", LOCAL_HOST) + .header("content-type", "application/x-www-form-urlencoded") + .body(Body::from( + "crate=kei-task&verb=x&kind=command&description=y", + )) + .unwrap(), + ) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::UNSUPPORTED_MEDIA_TYPE); +} + +/// FIX C (description injection): a newline in `description` could +/// escape the `sed` substitution inside `scripts/new-atom.sh` and +/// append a hostile `-e` expression. Must fail validation with 400. +#[tokio::test] +async fn post_description_with_newline_is_rejected() { + let app = server::app(); + // JSON escape for newline is \n literal in the string. + let body = r#"{"crate":"kei-task","verb":"noop","kind":"command","description":"foo\nevil"}"#; + let resp = app.oneshot(post_json("/forge", body)).await.unwrap(); + assert_eq!(resp.status(), StatusCode::BAD_REQUEST); + let bytes = to_bytes(resp.into_body(), 64 * 1024).await.unwrap(); + let json: Value = serde_json::from_slice(&bytes).unwrap(); + assert_eq!(json["success"], Value::Bool(false)); + let err = json["errors"][0].as_str().unwrap(); + assert!( + err.contains("description"), + "expected description error, got: {err}" + ); +} + +/// FIX (defence-in-depth): GET / must carry the four hardening +/// headers so an iframe / reflected-XSS pivot cannot escalate. +#[tokio::test] +async fn get_root_has_security_headers() { + let app = server::app(); + let resp = app.oneshot(get("/")).await.unwrap(); + assert_eq!(resp.status(), StatusCode::OK); + let h = resp.headers(); + assert!( + h.contains_key(header::CONTENT_SECURITY_POLICY), + "missing CSP header" + ); + assert!( + h.contains_key(header::X_CONTENT_TYPE_OPTIONS), + "missing X-Content-Type-Options" + ); + assert!( + h.contains_key(header::X_FRAME_OPTIONS), + "missing X-Frame-Options" + ); + assert!( + h.contains_key(header::REFERRER_POLICY), + "missing Referrer-Policy" + ); +}