Skip to content

fix(origin): handle null Origin header validation appropriately #216

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
AdamMagued wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(origin): handle null Origin header validation appropriately #216

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion csrf.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,15 @@ func (cs *csrf) ServeHTTP(w http.ResponseWriter, r *http.Request) {

// if we have an Origin header, check it against our allowlist
origin := r.Header.Get("Origin")
if origin != "" {
if origin == "null" {
if !slices.Contains(cs.opts.TrustedOrigins, "null") {
// Origin "null" is an opaque origin (RFC 6454 §7.3) sent from
// privacy-sensitive contexts. When not explicitly trusted in
// cs.opts.TrustedOrigins, treat it as absent so that Referer-based
// origin verification is evaluated below.
origin = ""
}
} else if origin != "" {
parsedOrigin, err := url.Parse(origin)
if err != nil {
r = envError(r, ErrBadOrigin)
Expand Down
183 changes: 183 additions & 0 deletions csrf_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -376,6 +376,7 @@ func TestProtectScenarios(t *testing.T) {
originUntrusted bool
originHTTP bool
originTrusted bool
originNull bool
secureRequest bool
refererTrusted bool
refererUntrusted bool
Expand Down Expand Up @@ -413,6 +414,17 @@ func TestProtectScenarios(t *testing.T) {
{
name: "cleartext POST without origin with invalid token reject",
},
{
name: "cleartext POST with null Origin & valid token pass",
originNull: true,
tokenValid: true,
want: true,
},
{
name: "cleartext POST with null Origin & invalid token reject",
originNull: true,
tokenInvalid: true,
},
{
name: "TLS POST with HTTP origin & no referer & valid token reject",
tokenValid: true,
Expand All @@ -424,19 +436,40 @@ func TestProtectScenarios(t *testing.T) {
secureRequest: true,
tokenValid: true,
},
{
name: "TLS POST with null Origin and without referer reject",
originNull: true,
secureRequest: true,
tokenValid: true,
},
{
name: "TLS POST without origin with untrusted referer reject",
secureRequest: true,
refererUntrusted: true,
tokenValid: true,
},
{
name: "TLS POST with null Origin and untrusted referer reject",
originNull: true,
secureRequest: true,
refererUntrusted: true,
tokenValid: true,
},
{
name: "TLS POST without origin with trusted referer & valid token pass",
secureRequest: true,
refererTrusted: true,
tokenValid: true,
want: true,
},
{
name: "TLS POST with null Origin and trusted referer & valid token pass",
originNull: true,
secureRequest: true,
refererTrusted: true,
tokenValid: true,
want: true,
},
{
name: "TLS POST without origin from _cleartext_ same domain referer with valid token reject",
secureRequest: true,
Expand Down Expand Up @@ -489,6 +522,8 @@ func TestProtectScenarios(t *testing.T) {
r.Header.Set("Origin", "https://www.gorillatoolkit.org")
case tt.originHTTP:
r.Header.Set("Origin", "http://www.gorillatoolkit.org")
case tt.originNull:
r.Header.Set("Origin", "null")
}

// Set the Referer header
Expand Down Expand Up @@ -542,3 +577,151 @@ func createRequest(method, path string, useTLS bool) *http.Request {
}
return r
}

func TestNullOrigin(t *testing.T) {
tests := []struct {
name string
secure bool
originHeader string
refererHeader string
trustedOrigins []string
validToken bool
wantStatus int
wantReason error
}{
{
name: "cleartext POST with Origin null and valid token succeeds",
secure: false,
originHeader: "null",
validToken: true,
wantStatus: http.StatusOK,
},
{
name: "cleartext POST with Origin null and invalid token rejected",
secure: false,
originHeader: "null",
validToken: false,
wantStatus: http.StatusForbidden,
wantReason: ErrBadToken,
},
{
name: "TLS POST with Origin null and no Referer rejected",
secure: true,
originHeader: "null",
validToken: true,
wantStatus: http.StatusForbidden,
wantReason: ErrNoReferer,
},
{
name: "TLS POST with Origin null and untrusted Referer rejected",
secure: true,
originHeader: "null",
refererHeader: "https://untrusted-site.com/form",
validToken: true,
wantStatus: http.StatusForbidden,
wantReason: ErrBadReferer,
},
{
name: "TLS POST with Origin null and HTTP downgrade Referer rejected",
secure: true,
originHeader: "null",
refererHeader: "http://www.gorillatoolkit.org/form",
validToken: true,
wantStatus: http.StatusForbidden,
wantReason: ErrBadReferer,
},
{
name: "TLS POST with Origin null and matching same-origin Referer succeeds",
secure: true,
originHeader: "null",
refererHeader: "https://www.gorillatoolkit.org/form",
validToken: true,
wantStatus: http.StatusOK,
},
{
name: "TLS POST with Origin null and external Referer in TrustedOrigins succeeds",
secure: true,
originHeader: "null",
refererHeader: "https://trusted-auth.org/redirect",
trustedOrigins: []string{"trusted-auth.org"},
validToken: true,
wantStatus: http.StatusOK,
},
{
name: "TLS POST with Origin null and TrustedOrigins configured with null succeeds without Referer",
secure: true,
originHeader: "null",
trustedOrigins: []string{"null"},
validToken: true,
wantStatus: http.StatusOK,
},
{
name: "TLS POST with Origin null, TrustedOrigins configured with null, and invalid token rejected",
secure: true,
originHeader: "null",
trustedOrigins: []string{"null"},
validToken: false,
wantStatus: http.StatusForbidden,
wantReason: ErrBadToken,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
var token string
var failureErr error

mux := http.NewServeMux()
mux.HandleFunc("/", func(_ http.ResponseWriter, r *http.Request) {
token = Token(r)
})
mux.HandleFunc("/submit", func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusOK)
})

opts := []Option{
ErrorHandler(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
failureErr = FailureReason(r)
http.Error(w, FailureReason(r).Error(), http.StatusForbidden)
})),
}
if len(tt.trustedOrigins) > 0 {
opts = append(opts, TrustedOrigins(tt.trustedOrigins))
}

p := Protect(testKey, opts...)(mux)

getReq := createRequest("GET", "/", tt.secure)
getRec := httptest.NewRecorder()
p.ServeHTTP(getRec, getReq)

postReq := createRequest("POST", "/submit", tt.secure)
setCookie(getRec, postReq)

if tt.originHeader != "" {
postReq.Header.Set("Origin", tt.originHeader)
}
if tt.refererHeader != "" {
postReq.Header.Set("Referer", tt.refererHeader)
}

if tt.validToken {
postReq.Header.Set("X-CSRF-Token", token)
} else {
postReq.Header.Set("X-CSRF-Token", "invalid-token-value")
}

postRec := httptest.NewRecorder()
p.ServeHTTP(postRec, postReq)

if postRec.Code != tt.wantStatus {
t.Fatalf("expected status %d, got %d (failure: %v)", tt.wantStatus, postRec.Code, failureErr)
}

if tt.wantReason != nil && failureErr != tt.wantReason {
t.Fatalf("expected failure reason %v, got %v", tt.wantReason, failureErr)
}
})
}
}

2 changes: 2 additions & 0 deletions options.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,8 @@ func CookieName(name string) Option {
// TrustedOrigins configures a set of origins (Referers) that are considered as trusted.
// This will allow cross-domain CSRF use-cases - e.g. where the front-end is served
// from a different domain than the API server - to correctly pass a CSRF check.
// Supplying "null" permits requests with an opaque Origin header (such as
// privacy-sensitive contexts defined in RFC 6454 §7.3) to be explicitly trusted.
//
// You should only provide origins you own or have full control over.
func TrustedOrigins(origins []string) Option {
Expand Down
Loading