Skip to content

CORS layer defaults to fully open (any origin/method/header) with no production guard #24

Description

@abayomicornelius

src/middleware/cors.rs::build_cors_layer combines AllowOrigin::any() (when allowed_origins is empty or contains "*") with AllowMethods::any() and AllowHeaders::any() unconditionally:

CorsLayer::new()
    .allow_origin(allow_origin)
    .allow_methods(AllowMethods::any())
    .allow_headers(AllowHeaders::any())

And src/config.rs::Config::from_env() defaults allowed_origins to "*" when ALLOWED_ORIGINS is unset entirely:

let allowed_origins_raw = env::var("ALLOWED_ORIGINS").unwrap_or_else(|_| "*".into());

There is no check anywhere in Config::from_env() or main.rs that config.is_production() (which the config struct already exposes) requires a non-wildcard allowed_origins list. A production deployment that simply forgets to set ALLOWED_ORIGINS silently gets a fully-open CORS policy for a financial API, with no warning logged.

Why it matters

  • AllowOrigin::any() + AllowMethods::any() + AllowHeaders::any() means any website can make cross-origin requests to every StellarSend endpoint from a victim's browser. Because auth here is Bearer-JWT-in-header (not cookies — see src/middleware/auth.rs::AuthUser, which reads Authorization: Bearer <token>), classic CSRF isn't directly possible (a malicious page can't make the browser attach the JWT automatically), but this still allows any origin to probe the API's behavior, and if any future feature introduces cookie-based session state, the existing wildcard CORS config would immediately become exploitable with zero additional code changes on this side.
  • tower_http::cors::CorsLayer will actually panic at runtime (not silently misbehave) if allow_credentials(true) is ever added alongside AllowOrigin::any() — the crate enforces the CORS spec's rule that credentialed requests cannot use a wildcard origin. Since nothing today calls .allow_credentials(true), this isn't currently triggered, but it's a landmine for whoever adds cookie/credentialed-request support later without realizing this file's default configuration is incompatible with it.
  • There's no environment-aware guard: is_production() exists on Config and is unused by build_cors_layer/main.rs for this purpose, meaning the "safe by default in dev, must opt into safety in prod" posture the codebase seems to intend elsewhere (e.g. JWT_SECRET's hard 32-char minimum) isn't applied consistently to CORS.

Reproduction / detection

  1. Run the server with no ALLOWED_ORIGINS set and APP_ENV=production — confirm build_cors_layer still returns AllowOrigin::any() with no warning logged, despite config.is_production() being true.
  2. curl -H "Origin: https://evil.example" -I http://localhost:PORT/api/accounts/GABC.../balances and confirm Access-Control-Allow-Origin: * (or a reflected origin) is returned regardless of environment.

Proposed fix

  • In Config::from_env() or main.rs startup, if config.is_production() and allowed_origins is empty or contains "*", either hard-fail startup (anyhow::bail!, matching the existing pattern for JWT_SECRET) or at minimum log a loud tracing::warn!/error! — hard-failing is more consistent with how this codebase already treats other production-safety gaps.
  • Narrow AllowMethods/AllowHeaders to the actual set this API uses (GET, POST and Authorization, Content-Type) rather than any(), independent of the origin-wildcard question — reflecting any() for headers/methods provides no functional benefit here since the API's real surface is small and known.

Edge cases

  • Preflight (OPTIONS) requests need Access-Control-Max-Age tuned sensibly once methods/headers are narrowed, to avoid excessive preflight round-trips from legitimate frontends.
  • If a future SDK/mobile client needs custom headers (e.g. X-Client-Version), the narrowed AllowHeaders list needs a documented process for extending it rather than reflexively reverting to any().

Testing strategy

  • Test asserting build_cors_layer returns a restrictive origin list when given a production-shaped config with empty allowed_origins (once the guard is added, this becomes "config load fails" rather than "CORS layer is restrictive" — test whichever behavior is chosen).
  • Test asserting a normal, explicit allowed_origins list in dev still works unmodified (no regression for the common local-dev "*" convenience).

Metadata

Metadata

Assignees

No one assigned

    Labels

    GrantFox OSSIssue tracked in GrantFox OSSMaybe RewardedIssue may be eligible for a GrantFox rewardOfficial Campaign | FWC26Campaign: Official Campaign | FWC26backendBackend service logicsecuritySecurity concernvery hardVery difficult / senior-level bounty issue

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions