mycelium/www_migrate_mycelium

Fork 0

Security audit #32

New issue

Closed

opened 2026-03-01 22:53:11 +00:00 by emre · 1 comment

emre commented

2026-03-01 22:53:11 +00:00

Owner

Security Audit Report — Mycelium Portal

Scope: Full-stack security review of mycportal — Rust/Axum backend, Dioxus/WASM frontend, deployment configs, CI/CD pipelines
Auditor: Opus 4.6 (Claude Code)

Executive Summary

Mycelium Portal is a full-stack blockchain interaction portal (TFChain + Hero Ledger/NEAR) with a Rust/Axum REST backend, a Dioxus/WASM single-page frontend, and Docker/Kubernetes deployment. The application handles real financial transactions (TFT token transfers, SPORE bridging).

Overall posture: Moderate risk. The architecture is sound — client-side signing, no database, Rust memory safety — but several exploitable issues exist in the API layer, financial calculations, and deployment configuration that must be addressed before production use.

Severity	Count
Critical	6
High	7
Medium	7
Low	5

Critical Findings

C1. CORS Allows All Origins

File: crates/backend/src/main.rs:54-57

let cors = CorsLayer::new()
    .allow_origin(Any)
    .allow_methods(Any)
    .allow_headers(Any);

Impact: Any website on the internet can make API requests to the backend. An attacker can host a malicious page that calls /api/transfer/prepare and /api/transfer/submit using a victim's session state. Since the prepare/submit flow relies only on session IDs (not origin-bound tokens), a cross-origin attacker can initiate transactions.

Recommendation: Restrict allow_origin to the actual deployment domains:

let cors = CorsLayer::new()
    .allow_origin(["https://migrate.projectmycelium.com".parse().unwrap()])
    .allow_methods([Method::GET, Method::POST])
    .allow_headers([header::CONTENT_TYPE]);

C2. No Signer Identity Verification on Transfer Submit

File: crates/backend/src/api.rs:373-472

The transfer_submit endpoint accepts any signer_account and never verifies it matches the from account stored in the PendingTransfer created during transfer_prepare.

// api.rs:377-380 — session is retrieved but `pending.from` is never checked
let (_session_id, pending) = state
    .pending_transfers
    .remove(&req.session_id)
    .ok_or_else(|| api_err(StatusCode::NOT_FOUND, "Session not found or expired"))?;
// req.signer_account is used directly without comparing to pending.from

The same issue exists in opt_out_v3_submit at line 562 — req.signer_account is not compared to pending.signer_account.

Impact: If an attacker obtains a valid session ID (e.g., via CORS abuse from C1, or network sniffing), they can submit a transaction with a different signer account.

Recommendation:

if req.signer_account != pending.from {
    return Err(api_err(StatusCode::BAD_REQUEST, "Signer does not match session"));
}

C3. Float-to-Integer Precision Loss in Financial Calculations

File: crates/backend/src/api.rs:311

let amount = (req.amount_tft * 10_000_000.0) as u128;

Impact: f64 cannot precisely represent all decimal values. For example:

0.1 * 10_000_000.0 may yield 999999.9999... which truncates to 999999 instead of 1000000
1.1 * 10_000_000.0 = 10999999.999... → 10999999 (loses 1 planck)

In a financial application, this means users can lose funds due to rounding. Over many transactions, the cumulative loss could be significant.

Recommendation: Parse the amount as a string, split on the decimal point, and compute planck units with integer arithmetic:

fn tft_string_to_planck(s: &str) -> Result<u128, String> {
    let parts: Vec<&str> = s.split('.').collect();
    let whole: u128 = parts[0].parse().map_err(|e| format!("{e}"))?;
    let frac: u128 = if parts.len() > 1 {
        let frac_str = format!("{:0<7}", &parts[1][..parts[1].len().min(7)]);
        frac_str.parse().map_err(|e| format!("{e}"))?
    } else { 0 };
    Ok(whole * 10_000_000 + frac)
}

C4. Missing Security Headers in Caddy Reverse Proxy

File: deploy/Caddyfile

The entire Caddyfile is:

{$DOMAIN:localhost} {
    reverse_proxy portal:11001
}

No security headers are set.

Impact:

No Content-Security-Policy: Enables XSS if any injection vector is found
No Strict-Transport-Security: HTTPS downgrade attacks possible
No X-Frame-Options: Clickjacking attacks — an attacker can iframe the portal and trick users into clicking transfer buttons
No X-Content-Type-Options: MIME sniffing attacks
No Referrer-Policy: Referrer leakage to external CDNs

Recommendation:

{$DOMAIN:localhost} {
    header {
        Strict-Transport-Security "max-age=63072000; includeSubDomains; preload"
        X-Frame-Options "DENY"
        X-Content-Type-Options "nosniff"
        Referrer-Policy "strict-origin-when-cross-origin"
        Permissions-Policy "camera=(), microphone=(), geolocation=()"
        Content-Security-Policy "default-src 'self'; script-src 'self' 'wasm-unsafe-eval'; style-src 'self' 'unsafe-inline' https://cdn.jsdelivr.net https://fonts.googleapis.com; font-src 'self' https://cdn.jsdelivr.net https://fonts.gstatic.com; connect-src 'self' wss://tfchain.grid.tf wss://tfchain.test.grid.tf wss://tfchain.dev.grid.tf"
    }
    reverse_proxy portal:11001
}

C5. Container Runs as Root

File: Dockerfile:22-39

The runtime stage has no USER directive. The application runs as root (UID 0).

Impact: If the application is compromised, the attacker gains root privileges inside the container, with potential to escape the container namespace, modify mounted volumes, or install persistence mechanisms.

Recommendation:

RUN useradd -r -s /bin/false -u 1001 appuser
USER appuser

C6. SSH MITM Vulnerability in Deployment Pipeline

File: .forgejo/workflows/deploy-staging.yml:74

SSH_OPTS="... -o StrictHostKeyChecking=accept-new ..."
ssh $SSH_OPTS root@staging.hub.projectmycelium.com "..."

Impact: StrictHostKeyChecking=accept-new accepts any host key on first connection. An attacker performing a man-in-the-middle attack on the CI runner's network gets root shell access to the staging server. This is compounded by deploying as root.

Recommendation: Pre-populate known_hosts with the server's fingerprint in the workflow, and deploy as a dedicated non-root user with minimal sudo privileges.

High Findings

H1. No Rate Limiting on Any Endpoint

Files: crates/backend/src/main.rs, crates/backend/src/api.rs

All 22+ API endpoints are completely unprotected against abuse.

Impact:

/api/transfer/prepare can be spammed to fill the DashMap with millions of pending sessions (memory exhaustion DoS)
/api/balance/{account_id} can be used to enumerate all on-chain accounts
No protection against automated transaction submission

Recommendation: Add tower::limit::RateLimitLayer or a per-IP rate limiter. At minimum, rate-limit the prepare/submit endpoints.

H2. Session ID Memory Exhaustion (DoS)

File: crates/backend/src/api.rs:352-363 and crates/backend/src/state.rs:90-94

Each call to /api/transfer/prepare inserts into an unbounded DashMap. Cleanup runs every 30 seconds and retains entries for 5 minutes. An attacker sending 100k requests/second for 30 seconds creates 3 million entries before cleanup fires.

Impact: Out-of-memory crash, denial of service.

Recommendation: Add a maximum size to the pending maps. Reject new sessions when the limit is reached. Add per-IP rate limiting.

H3. Mnemonic Stored as Plaintext in WASM Memory

File: crates/frontend/src/signing.rs:15-19

pub struct WalletState {
    keypair: subxt_signer::sr25519::Keypair,
    account_id_ss58: String,
    mnemonic_phrase: String,  // plaintext in WASM linear memory
}

WASM linear memory is inspectable via browser DevTools (WebAssembly.Memory). The mnemonic — which controls all funds — persists for the entire session.

Recommendation: Use the zeroize crate to clear the mnemonic after deriving the keypair, or at minimum document this as a known risk. Consider not storing the mnemonic in WalletState at all after initial derivation.

H4. No NaN/Infinity Validation on Transfer Amount

File: crates/backend/src/api.rs:311-317

let amount = (req.amount_tft * 10_000_000.0) as u128;
if amount == 0 {
    return Err(api_err(StatusCode::BAD_REQUEST, "Amount must be greater than 0"));
}

If amount_tft is NaN or Infinity (both valid JSON floats), the as u128 cast produces 0 for NaN (passes the check as 0, but blocked) and a large value for Infinity. f64::INFINITY as u128 is platform-dependent.

Impact: Undefined behavior in financial calculations.

Recommendation: Add if !req.amount_tft.is_finite() || req.amount_tft <= 0.0 { return Err(...) }

H5. Unpinned Git Dependency (Supply Chain Risk)

File: crates/backend/Cargo.toml:8

mycelium_tfchain_client = { git = "https://forge.ourworld.tf/geomind_code/tfchain_client.git" }

No rev, tag, or branch specified. A compromised upstream repo could inject malicious code that runs in your backend with full access to the RPC client and pending transactions.

Recommendation: Pin to a specific commit: rev = "abc1234...". Also pin the heroledger_gateway_client dependency in the frontend.

H6. Missing Kubernetes Security Context

File: deploy/k8s/deployment.yml

The deployment lacks all security context specifications:

No runAsNonRoot: true
No readOnlyRootFilesystem: true
No allowPrivilegeEscalation: false
No capabilities: drop: [ALL]
No NetworkPolicy (lateral movement possible within cluster)

Recommendation: Add comprehensive securityContext to both Pod and Container specs.

H7. Unpinned Docker Base Images

Files: Dockerfile:1,22, deploy/docker-compose.yml:3, deploy/k8s/deployment.yml:19

FROM rust:latest — non-deterministic build
FROM debian:bookworm-slim — no digest pin
image: .../www-migrate-mycelium:latest — both Docker Compose and Kubernetes

Impact: Supply chain attack vector. A compromised upstream image silently enters production. Builds are not reproducible.

Recommendation: Pin all images to specific digests: rust:1.77-bookworm@sha256:...

Medium Findings

M1. Error Messages Leak Internal Details

Files: crates/backend/src/api.rs (multiple locations)

Error messages like format!("Chain error: {e}") and format!("Failed to create partial tx: {e}") expose internal chain RPC errors, Substrate metadata details, and infrastructure information to API consumers.

Recommendation: Return generic error messages to clients. Log detailed errors server-side only.

M2. No CSRF Protection

File: crates/frontend/src/api.rs

All API calls use simple POST/GET with only a Content-Type: application/json header. No CSRF tokens, no custom headers for same-origin validation. Combined with the open CORS policy (C1), this makes cross-site request forgery trivial.

Recommendation: After fixing CORS (C1), add a custom header requirement (e.g., X-Requested-With) that simple CORS requests cannot send.

M3. Explorer `farm_ids` Path Parameter Not Validated

File: crates/backend/src/api.rs:691-700

Path(farm_ids): Path<String>,
// ...
state.explorer.list_nodes_by_farm_ids(&farm_ids).await

The farm_ids string is passed directly to the Explorer API without validation. Depending on how the Explorer constructs its HTTP query, this could enable parameter injection (e.g., extra query parameters or path traversal).

Recommendation: Parse farm_ids as a comma-separated list of integers and reject invalid input.

M4. No Content Security Policy from Application

File: crates/frontend/src/main.rs:147-170

The frontend loads external resources from CDNs (Bootstrap CSS, Bootstrap Icons, Google Fonts) without Subresource Integrity (SRI) hashes. A compromised CDN could inject malicious CSS.

Recommendation: Add SRI hashes to all external resource links and enforce CSP from the server (see C4).

M5. CI/CD Input Injection Risk

File: .forgejo/workflows/build-container.yml:62-76

User-provided input from github.event.inputs.version is used directly in Docker tags without sanitization:

VERSION="${{ github.event.inputs.version }}"
TAGS="${REGISTRY}/${OWNER}/${IMAGE}:${VERSION}"

Recommendation: Validate version input with a regex: ^[a-zA-Z0-9._-]+$

M6. Hardcoded Test Password in E2E Tests

File: e2e/tests/vault-flow.spec.ts:7

const VAULT_PASSWORD = "testpassword123";

Recommendation: Use an environment variable: process.env.VAULT_TEST_PASSWORD || "testpassword123"

M7. Single Token for Multiple CI/CD Purposes

File: .forgejo/workflows/build-container.yml

FORGEJO_TOKEN is used for git cloning, Docker registry authentication, AND release API calls. Compromising one system compromises all three.

Recommendation: Create separate tokens with minimal required permissions for each purpose.

Low Findings

L1. No Request Timeout on Backend

File: crates/backend/src/main.rs:103

axum::serve has no timeout configuration. A slow client can hold connections open indefinitely, potentially exhausting file descriptors.

Recommendation: Add tower::timeout::TimeoutLayer.

L2. Health Endpoint Exposes Internal State

File: crates/backend/src/api.rs:17-32

/api/health returns the latest block number, network name, and connection status without authentication — useful reconnaissance data.

L3. Integer Truncation in Explorer Responses

File: crates/backend/src/api.rs:672-674

id: f.id as u32,
twin_id: f.twin_id as u32,

If the Explorer returns values exceeding u32::MAX, these silently truncate.

L4. No Vault Brute-Force Protection

File: crates/frontend/src/components/login.rs:269-300

The vault unlock flow has no rate limiting or lockout after failed password attempts. An attacker with access to the browser can brute-force the vault password offline since the encrypted data is in localStorage.

Recommendation: Use a slow KDF (Argon2id) with high cost parameters for vault encryption. Document that vault security depends on password strength.

L5. Logout Does Not Clear Sensitive State

File: crates/frontend/src/components/login.rs:161-165

onclick: move |_| {
    wallet.set(None);
    hero_ledger.set(HeroLedgerAccount(None));
    // ...
},

Setting signals to None does not zeroize the previous value from WASM memory. The mnemonic and keypair may remain in memory after logout until garbage collected or overwritten.

What's Done Well

Area	Assessment
Client-side signing	Mnemonics never sent to server. Signing happens in WASM.
Two-phase transaction flow	Prepare → sign → submit pattern is architecturally correct
Session expiry	5-minute TTL with periodic cleanup
Rust + WASM	Memory safety eliminates buffer overflows, use-after-free
No database	Stateless RPC proxy eliminates SQL injection entirely
Vault encryption	AES-256-GCM for browser-side mnemonic storage
SR25519 cryptography	Well-audited `subxt-signer` library
Git hygiene	`.gitignore` properly excludes `.env`, no secrets committed
CI pipeline	Clippy linting, tests, formatting checks on every PR
Input validation	Account IDs validated as SS58, memo validated as 64-char hex
Password strength	Vault passwords require 8+ chars with letters and numbers

Priority Action Items

Before Publishing (P0)

#	Issue	Fix	Effort
1	CORS allows all origins (C1)	Restrict to deployment domains	5 min
2	No signer verification on submit (C2)	Compare `req.signer_account` to `pending.from`	5 min
3	Float precision loss (C3)	Use string-based integer math for planck conversion	30 min
4	Add `is_finite()` check (H4)	Validate amount is finite and positive	5 min
5	Security headers in Caddy (C4)	Add CSP, HSTS, X-Frame-Options, etc.	15 min

Before Production (P1)

#	Issue	Fix	Effort
6	Rate limiting (H1, H2)	Add `tower` rate limit middleware	1 hr
7	Container runs as root (C5)	Add non-root user to Dockerfile	10 min
8	Pin git dependencies (H5)	Add `rev = "..."` to Cargo.toml	5 min
9	Pin Docker base images (H7)	Use digest-pinned image references	15 min
10	Fix registration bug (F1)	Wire email/name to gateway or remove fields	30 min

Improvement (P2)

#	Issue	Fix	Effort
11	Sanitize error messages (M1)	Generic client errors, detailed server logs	1 hr
12	Kubernetes security context (H6)	Add securityContext + NetworkPolicy	30 min
13	Validate `farm_ids` parameter (M3)	Parse as comma-separated integers	15 min
14	Add SRI hashes to CDN resources (M4)	Compute and add integrity attributes	15 min
15	Zeroize mnemonic in memory (H3)	Add `zeroize` crate, clear after derivation	1 hr
16	SSH host key verification (C6)	Pre-populate known_hosts in CI	15 min

Methodology

This audit was conducted through static analysis of all source code, configuration files, CI/CD pipelines, and deployment manifests. The following files were reviewed in full:

crates/backend/src/main.rs (104 lines)
crates/backend/src/api.rs (822 lines)
crates/backend/src/state.rs (95 lines)
crates/shared/src/lib.rs (255 lines)
crates/frontend/src/main.rs (1102 lines)
crates/frontend/src/signing.rs (172 lines)
crates/frontend/src/api.rs (117 lines)
crates/frontend/src/heroledger.rs (296 lines)
crates/frontend/src/config.rs (8 lines)
crates/frontend/src/components/login.rs (606 lines)
crates/frontend/src/components/transfer.rs (635 lines)
crates/frontend/src/components/balance.rs (84 lines)
crates/frontend/src/components/sidebar.rs (208 lines)
crates/frontend/src/pages/prepare_pre_register.rs (393 lines)
Dockerfile
deploy/Caddyfile, deploy/docker-compose.yml
deploy/k8s/deployment.yml, deploy/k8s/ingress.yml, deploy/k8s/service.yml, deploy/k8s/configmap.yml
.forgejo/workflows/test.yml, build-container.yml, deploy-staging.yml
e2e/tests/vault-flow.spec.ts
All Cargo.toml files and .gitignore

No dynamic testing, penetration testing, or dependency vulnerability scanning (e.g., cargo audit) was performed. These are recommended as follow-up activities.

# Security Audit Report — Mycelium Portal **Scope:** Full-stack security review of `mycportal` — Rust/Axum backend, Dioxus/WASM frontend, deployment configs, CI/CD pipelines **Auditor:** Opus 4.6 (Claude Code) --- ## Executive Summary Mycelium Portal is a full-stack blockchain interaction portal (TFChain + Hero Ledger/NEAR) with a Rust/Axum REST backend, a Dioxus/WASM single-page frontend, and Docker/Kubernetes deployment. The application handles **real financial transactions** (TFT token transfers, SPORE bridging). **Overall posture: Moderate risk.** The architecture is sound — client-side signing, no database, Rust memory safety — but several exploitable issues exist in the API layer, financial calculations, and deployment configuration that must be addressed before production use. | Severity | Count | |----------|-------| | Critical | 6 | | High | 7 | | Medium | 7 | | Low | 5 | --- ## Critical Findings ### C1. CORS Allows All Origins **File:** `crates/backend/src/main.rs:54-57` ```rust let cors = CorsLayer::new() .allow_origin(Any) .allow_methods(Any) .allow_headers(Any); ``` **Impact:** Any website on the internet can make API requests to the backend. An attacker can host a malicious page that calls `/api/transfer/prepare` and `/api/transfer/submit` using a victim's session state. Since the prepare/submit flow relies only on session IDs (not origin-bound tokens), a cross-origin attacker can initiate transactions. **Recommendation:** Restrict `allow_origin` to the actual deployment domains: ```rust let cors = CorsLayer::new() .allow_origin(["https://migrate.projectmycelium.com".parse().unwrap()]) .allow_methods([Method::GET, Method::POST]) .allow_headers([header::CONTENT_TYPE]); ``` --- ### C2. No Signer Identity Verification on Transfer Submit **File:** `crates/backend/src/api.rs:373-472` The `transfer_submit` endpoint accepts any `signer_account` and never verifies it matches the `from` account stored in the `PendingTransfer` created during `transfer_prepare`. ```rust // api.rs:377-380 — session is retrieved but `pending.from` is never checked let (_session_id, pending) = state .pending_transfers .remove(&req.session_id) .ok_or_else(|| api_err(StatusCode::NOT_FOUND, "Session not found or expired"))?; // req.signer_account is used directly without comparing to pending.from ``` The same issue exists in `opt_out_v3_submit` at line 562 — `req.signer_account` is not compared to `pending.signer_account`. **Impact:** If an attacker obtains a valid session ID (e.g., via CORS abuse from C1, or network sniffing), they can submit a transaction with a different signer account. **Recommendation:** ```rust if req.signer_account != pending.from { return Err(api_err(StatusCode::BAD_REQUEST, "Signer does not match session")); } ``` --- ### C3. Float-to-Integer Precision Loss in Financial Calculations **File:** `crates/backend/src/api.rs:311` ```rust let amount = (req.amount_tft * 10_000_000.0) as u128; ``` **Impact:** `f64` cannot precisely represent all decimal values. For example: - `0.1 * 10_000_000.0` may yield `999999.9999...` which truncates to `999999` instead of `1000000` - `1.1 * 10_000_000.0` = `10999999.999...` → `10999999` (loses 1 planck) In a financial application, this means users can **lose funds due to rounding**. Over many transactions, the cumulative loss could be significant. **Recommendation:** Parse the amount as a string, split on the decimal point, and compute planck units with integer arithmetic: ```rust fn tft_string_to_planck(s: &str) -> Result<u128, String> { let parts: Vec<&str> = s.split('.').collect(); let whole: u128 = parts[0].parse().map_err(|e| format!("{e}"))?; let frac: u128 = if parts.len() > 1 { let frac_str = format!("{:0<7}", &parts[1][..parts[1].len().min(7)]); frac_str.parse().map_err(|e| format!("{e}"))? } else { 0 }; Ok(whole * 10_000_000 + frac) } ``` --- ### C4. Missing Security Headers in Caddy Reverse Proxy **File:** `deploy/Caddyfile` The entire Caddyfile is: ``` {$DOMAIN:localhost} { reverse_proxy portal:11001 } ``` No security headers are set. **Impact:** - **No Content-Security-Policy:** Enables XSS if any injection vector is found - **No Strict-Transport-Security:** HTTPS downgrade attacks possible - **No X-Frame-Options:** Clickjacking attacks — an attacker can iframe the portal and trick users into clicking transfer buttons - **No X-Content-Type-Options:** MIME sniffing attacks - **No Referrer-Policy:** Referrer leakage to external CDNs **Recommendation:** ``` {$DOMAIN:localhost} { header { Strict-Transport-Security "max-age=63072000; includeSubDomains; preload" X-Frame-Options "DENY" X-Content-Type-Options "nosniff" Referrer-Policy "strict-origin-when-cross-origin" Permissions-Policy "camera=(), microphone=(), geolocation=()" Content-Security-Policy "default-src 'self'; script-src 'self' 'wasm-unsafe-eval'; style-src 'self' 'unsafe-inline' https://cdn.jsdelivr.net https://fonts.googleapis.com; font-src 'self' https://cdn.jsdelivr.net https://fonts.gstatic.com; connect-src 'self' wss://tfchain.grid.tf wss://tfchain.test.grid.tf wss://tfchain.dev.grid.tf" } reverse_proxy portal:11001 } ``` --- ### C5. Container Runs as Root **File:** `Dockerfile:22-39` The runtime stage has no `USER` directive. The application runs as `root` (UID 0). **Impact:** If the application is compromised, the attacker gains root privileges inside the container, with potential to escape the container namespace, modify mounted volumes, or install persistence mechanisms. **Recommendation:** ```dockerfile RUN useradd -r -s /bin/false -u 1001 appuser USER appuser ``` --- ### C6. SSH MITM Vulnerability in Deployment Pipeline **File:** `.forgejo/workflows/deploy-staging.yml:74` ```bash SSH_OPTS="... -o StrictHostKeyChecking=accept-new ..." ssh $SSH_OPTS root@staging.hub.projectmycelium.com "..." ``` **Impact:** `StrictHostKeyChecking=accept-new` accepts any host key on first connection. An attacker performing a man-in-the-middle attack on the CI runner's network gets **root shell access** to the staging server. This is compounded by deploying as `root`. **Recommendation:** Pre-populate `known_hosts` with the server's fingerprint in the workflow, and deploy as a dedicated non-root user with minimal sudo privileges. --- ## High Findings ### H1. No Rate Limiting on Any Endpoint **Files:** `crates/backend/src/main.rs`, `crates/backend/src/api.rs` All 22+ API endpoints are completely unprotected against abuse. **Impact:** - `/api/transfer/prepare` can be spammed to fill the `DashMap` with millions of pending sessions (memory exhaustion DoS) - `/api/balance/{account_id}` can be used to enumerate all on-chain accounts - No protection against automated transaction submission **Recommendation:** Add `tower::limit::RateLimitLayer` or a per-IP rate limiter. At minimum, rate-limit the prepare/submit endpoints. --- ### H2. Session ID Memory Exhaustion (DoS) **File:** `crates/backend/src/api.rs:352-363` and `crates/backend/src/state.rs:90-94` Each call to `/api/transfer/prepare` inserts into an unbounded `DashMap`. Cleanup runs every 30 seconds and retains entries for 5 minutes. An attacker sending 100k requests/second for 30 seconds creates 3 million entries before cleanup fires. **Impact:** Out-of-memory crash, denial of service. **Recommendation:** Add a maximum size to the pending maps. Reject new sessions when the limit is reached. Add per-IP rate limiting. --- ### H3. Mnemonic Stored as Plaintext in WASM Memory **File:** `crates/frontend/src/signing.rs:15-19` ```rust pub struct WalletState { keypair: subxt_signer::sr25519::Keypair, account_id_ss58: String, mnemonic_phrase: String, // plaintext in WASM linear memory } ``` WASM linear memory is inspectable via browser DevTools (`WebAssembly.Memory`). The mnemonic — which controls all funds — persists for the entire session. **Recommendation:** Use the `zeroize` crate to clear the mnemonic after deriving the keypair, or at minimum document this as a known risk. Consider not storing the mnemonic in `WalletState` at all after initial derivation. --- ### H4. No NaN/Infinity Validation on Transfer Amount **File:** `crates/backend/src/api.rs:311-317` ```rust let amount = (req.amount_tft * 10_000_000.0) as u128; if amount == 0 { return Err(api_err(StatusCode::BAD_REQUEST, "Amount must be greater than 0")); } ``` If `amount_tft` is `NaN` or `Infinity` (both valid JSON floats), the `as u128` cast produces `0` for NaN (passes the check as 0, but blocked) and a large value for Infinity. `f64::INFINITY as u128` is platform-dependent. **Impact:** Undefined behavior in financial calculations. **Recommendation:** Add `if !req.amount_tft.is_finite() || req.amount_tft <= 0.0 { return Err(...) }` --- ### H5. Unpinned Git Dependency (Supply Chain Risk) **File:** `crates/backend/Cargo.toml:8` ```toml mycelium_tfchain_client = { git = "https://forge.ourworld.tf/geomind_code/tfchain_client.git" } ``` No `rev`, `tag`, or `branch` specified. A compromised upstream repo could inject malicious code that runs in your backend with full access to the RPC client and pending transactions. **Recommendation:** Pin to a specific commit: `rev = "abc1234..."`. Also pin the `heroledger_gateway_client` dependency in the frontend. --- ### H6. Missing Kubernetes Security Context **File:** `deploy/k8s/deployment.yml` The deployment lacks all security context specifications: - No `runAsNonRoot: true` - No `readOnlyRootFilesystem: true` - No `allowPrivilegeEscalation: false` - No `capabilities: drop: [ALL]` - No `NetworkPolicy` (lateral movement possible within cluster) **Recommendation:** Add comprehensive `securityContext` to both Pod and Container specs. --- ### H7. Unpinned Docker Base Images **Files:** `Dockerfile:1,22`, `deploy/docker-compose.yml:3`, `deploy/k8s/deployment.yml:19` - `FROM rust:latest` — non-deterministic build - `FROM debian:bookworm-slim` — no digest pin - `image: .../www-migrate-mycelium:latest` — both Docker Compose and Kubernetes **Impact:** Supply chain attack vector. A compromised upstream image silently enters production. Builds are not reproducible. **Recommendation:** Pin all images to specific digests: `rust:1.77-bookworm@sha256:...` --- ## Medium Findings ### M1. Error Messages Leak Internal Details **Files:** `crates/backend/src/api.rs` (multiple locations) Error messages like `format!("Chain error: {e}")` and `format!("Failed to create partial tx: {e}")` expose internal chain RPC errors, Substrate metadata details, and infrastructure information to API consumers. **Recommendation:** Return generic error messages to clients. Log detailed errors server-side only. --- ### M2. No CSRF Protection **File:** `crates/frontend/src/api.rs` All API calls use simple POST/GET with only a `Content-Type: application/json` header. No CSRF tokens, no custom headers for same-origin validation. Combined with the open CORS policy (C1), this makes cross-site request forgery trivial. **Recommendation:** After fixing CORS (C1), add a custom header requirement (e.g., `X-Requested-With`) that simple CORS requests cannot send. --- ### M3. Explorer `farm_ids` Path Parameter Not Validated **File:** `crates/backend/src/api.rs:691-700` ```rust Path(farm_ids): Path<String>, // ... state.explorer.list_nodes_by_farm_ids(&farm_ids).await ``` The `farm_ids` string is passed directly to the Explorer API without validation. Depending on how the Explorer constructs its HTTP query, this could enable parameter injection (e.g., extra query parameters or path traversal). **Recommendation:** Parse `farm_ids` as a comma-separated list of integers and reject invalid input. --- ### M4. No Content Security Policy from Application **File:** `crates/frontend/src/main.rs:147-170` The frontend loads external resources from CDNs (Bootstrap CSS, Bootstrap Icons, Google Fonts) without Subresource Integrity (SRI) hashes. A compromised CDN could inject malicious CSS. **Recommendation:** Add SRI hashes to all external resource links and enforce CSP from the server (see C4). --- ### M5. CI/CD Input Injection Risk **File:** `.forgejo/workflows/build-container.yml:62-76` User-provided input from `github.event.inputs.version` is used directly in Docker tags without sanitization: ```bash VERSION="${{ github.event.inputs.version }}" TAGS="${REGISTRY}/${OWNER}/${IMAGE}:${VERSION}" ``` **Recommendation:** Validate version input with a regex: `^[a-zA-Z0-9._-]+$` --- ### M6. Hardcoded Test Password in E2E Tests **File:** `e2e/tests/vault-flow.spec.ts:7` ```typescript const VAULT_PASSWORD = "testpassword123"; ``` **Recommendation:** Use an environment variable: `process.env.VAULT_TEST_PASSWORD || "testpassword123"` --- ### M7. Single Token for Multiple CI/CD Purposes **File:** `.forgejo/workflows/build-container.yml` `FORGEJO_TOKEN` is used for git cloning, Docker registry authentication, AND release API calls. Compromising one system compromises all three. **Recommendation:** Create separate tokens with minimal required permissions for each purpose. --- ## Low Findings ### L1. No Request Timeout on Backend **File:** `crates/backend/src/main.rs:103` `axum::serve` has no timeout configuration. A slow client can hold connections open indefinitely, potentially exhausting file descriptors. **Recommendation:** Add `tower::timeout::TimeoutLayer`. --- ### L2. Health Endpoint Exposes Internal State **File:** `crates/backend/src/api.rs:17-32` `/api/health` returns the latest block number, network name, and connection status without authentication — useful reconnaissance data. --- ### L3. Integer Truncation in Explorer Responses **File:** `crates/backend/src/api.rs:672-674` ```rust id: f.id as u32, twin_id: f.twin_id as u32, ``` If the Explorer returns values exceeding `u32::MAX`, these silently truncate. --- ### L4. No Vault Brute-Force Protection **File:** `crates/frontend/src/components/login.rs:269-300` The vault unlock flow has no rate limiting or lockout after failed password attempts. An attacker with access to the browser can brute-force the vault password offline since the encrypted data is in `localStorage`. **Recommendation:** Use a slow KDF (Argon2id) with high cost parameters for vault encryption. Document that vault security depends on password strength. --- ### L5. Logout Does Not Clear Sensitive State **File:** `crates/frontend/src/components/login.rs:161-165` ```rust onclick: move |_| { wallet.set(None); hero_ledger.set(HeroLedgerAccount(None)); // ... }, ``` Setting signals to `None` does not zeroize the previous value from WASM memory. The mnemonic and keypair may remain in memory after logout until garbage collected or overwritten. --- ## What's Done Well | Area | Assessment | |------|-----------| | **Client-side signing** | Mnemonics never sent to server. Signing happens in WASM. | | **Two-phase transaction flow** | Prepare → sign → submit pattern is architecturally correct | | **Session expiry** | 5-minute TTL with periodic cleanup | | **Rust + WASM** | Memory safety eliminates buffer overflows, use-after-free | | **No database** | Stateless RPC proxy eliminates SQL injection entirely | | **Vault encryption** | AES-256-GCM for browser-side mnemonic storage | | **SR25519 cryptography** | Well-audited `subxt-signer` library | | **Git hygiene** | `.gitignore` properly excludes `.env`, no secrets committed | | **CI pipeline** | Clippy linting, tests, formatting checks on every PR | | **Input validation** | Account IDs validated as SS58, memo validated as 64-char hex | | **Password strength** | Vault passwords require 8+ chars with letters and numbers | --- ## Priority Action Items ### Before Publishing (P0) | # | Issue | Fix | Effort | |---|-------|-----|--------| | 1 | CORS allows all origins (C1) | Restrict to deployment domains | 5 min | | 2 | No signer verification on submit (C2) | Compare `req.signer_account` to `pending.from` | 5 min | | 3 | Float precision loss (C3) | Use string-based integer math for planck conversion | 30 min | | 4 | Add `is_finite()` check (H4) | Validate amount is finite and positive | 5 min | | 5 | Security headers in Caddy (C4) | Add CSP, HSTS, X-Frame-Options, etc. | 15 min | ### Before Production (P1) | # | Issue | Fix | Effort | |---|-------|-----|--------| | 6 | Rate limiting (H1, H2) | Add `tower` rate limit middleware | 1 hr | | 7 | Container runs as root (C5) | Add non-root user to Dockerfile | 10 min | | 8 | Pin git dependencies (H5) | Add `rev = "..."` to Cargo.toml | 5 min | | 9 | Pin Docker base images (H7) | Use digest-pinned image references | 15 min | | 10 | Fix registration bug (F1) | Wire email/name to gateway or remove fields | 30 min | ### Improvement (P2) | # | Issue | Fix | Effort | |---|-------|-----|--------| | 11 | Sanitize error messages (M1) | Generic client errors, detailed server logs | 1 hr | | 12 | Kubernetes security context (H6) | Add securityContext + NetworkPolicy | 30 min | | 13 | Validate `farm_ids` parameter (M3) | Parse as comma-separated integers | 15 min | | 14 | Add SRI hashes to CDN resources (M4) | Compute and add integrity attributes | 15 min | | 15 | Zeroize mnemonic in memory (H3) | Add `zeroize` crate, clear after derivation | 1 hr | | 16 | SSH host key verification (C6) | Pre-populate known_hosts in CI | 15 min | --- ## Methodology This audit was conducted through static analysis of all source code, configuration files, CI/CD pipelines, and deployment manifests. The following files were reviewed in full: - `crates/backend/src/main.rs` (104 lines) - `crates/backend/src/api.rs` (822 lines) - `crates/backend/src/state.rs` (95 lines) - `crates/shared/src/lib.rs` (255 lines) - `crates/frontend/src/main.rs` (1102 lines) - `crates/frontend/src/signing.rs` (172 lines) - `crates/frontend/src/api.rs` (117 lines) - `crates/frontend/src/heroledger.rs` (296 lines) - `crates/frontend/src/config.rs` (8 lines) - `crates/frontend/src/components/login.rs` (606 lines) - `crates/frontend/src/components/transfer.rs` (635 lines) - `crates/frontend/src/components/balance.rs` (84 lines) - `crates/frontend/src/components/sidebar.rs` (208 lines) - `crates/frontend/src/pages/prepare_pre_register.rs` (393 lines) - `Dockerfile` - `deploy/Caddyfile`, `deploy/docker-compose.yml` - `deploy/k8s/deployment.yml`, `deploy/k8s/ingress.yml`, `deploy/k8s/service.yml`, `deploy/k8s/configmap.yml` - `.forgejo/workflows/test.yml`, `build-container.yml`, `deploy-staging.yml` - `e2e/tests/vault-flow.spec.ts` - All `Cargo.toml` files and `.gitignore` No dynamic testing, penetration testing, or dependency vulnerability scanning (e.g., `cargo audit`) was performed. These are recommended as follow-up activities.