security: harden CSRF Origin check and CORS credentials handling (#5, #16)#585
Merged
lakhansamani merged 1 commit intomainfrom Apr 7, 2026
Merged
security: harden CSRF Origin check and CORS credentials handling (#5, #16)#585lakhansamani merged 1 commit intomainfrom
lakhansamani merged 1 commit intomainfrom
Conversation
…16) Two related issues: #5 — CSRF bypass with wildcard AllowedOrigins The CSRF middleware called validators.IsValidOrigin to check the Origin header, but IsValidOrigin returns true for ANY origin when AllowedOrigins is the wildcard ["*"] (the default). It also skipped the check entirely when the Origin header was missing, accepting an X-Requested-With trick. An attacker could send a state-changing POST from a malicious page without Origin and pass the check. Fix: - Require Origin OR Referer on every state-changing method. Reject 403 with "Origin or Referer header is required" if both are missing. - A CSRF-specific csrfOriginAllowed() validates the origin. When AllowedOrigins == ["*"], it falls back to same-origin enforcement (Origin host must equal request Host) — wildcard CORS does not mean wildcard CSRF. With an explicit allowlist it defers to IsValidOrigin. - The custom-header check (application/json or X-Requested-With) is still required as a second layer. #16 — CORS wildcard with credentials The CORS middleware already correctly avoids setting Access-Control-Allow-Credentials when origin is "*" or empty, so credentialed cross-origin requests are not actually permitted in wildcard mode. To stop operators from accidentally relying on this default in production, log a clear startup warning when AllowedOrigins contains "*", recommending an explicit allowlist.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two related issues:
#5 — CSRF bypass with wildcard
AllowedOriginsThe CSRF middleware called
validators.IsValidOriginto check theOriginheader, butIsValidOriginreturnstruefor ANY origin whenAllowedOrigins == ["*"](the default). It also skipped the check entirely whenOriginwas missing, accepting anX-Requested-Withtrick. Combined, an attacker could send a state-changing POST from a malicious page withoutOriginand pass.#16 — CORS wildcard with credentials
The CORS middleware already correctly avoids setting
Access-Control-Allow-Credentialswhen origin is*or empty, so credentialed cross-origin requests aren't actually permitted in wildcard mode. Operators can still misread the default as "credentialed wildcard works" and rely on it.Changes
OriginORRefereron every state-changing method. Reject with403 Origin or Referer header is requiredif both are missing.csrfOriginAllowed()validates the origin. WhenAllowedOrigins == ["*"], it falls back to same-origin enforcement (origin host must equal request host) — wildcard CORS does not mean wildcard CSRF. With an explicit allowlist it defers toIsValidOrigin.application/jsonorX-Requested-With) is still required as a second layer.AllowedOriginscontains*recommending an explicit allowlist for production.Test plan
go build ./...TEST_DBS=sqlite go test ./internal/integration_tests/