Skip to content

Commit

Permalink
[bugfix] Don't return the origin header when configured to * (#116)
Browse files Browse the repository at this point in the history
There's no reason to allow for a server to reflect all origin headers.
This has caused numerous security problems in the past.
 - cyu/rack-cors#126
 - https://nodesecurity.io/advisories/148
 - captncraig/cors@cc1cf75

Some helpful blog posts on the topic:
 - https://ejj.io/misconfigured-cors/
 - http://blog.portswigger.net/2016/10/exploiting-cors-misconfigurations-for.html
  • Loading branch information
ejcx authored and elithrar committed Nov 1, 2017
1 parent a4d79d4 commit 9066371
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 2 deletions.
12 changes: 11 additions & 1 deletion cors.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,17 @@ func (ch *cors) ServeHTTP(w http.ResponseWriter, r *http.Request) {
w.Header().Set(corsVaryHeader, corsOriginHeader)
}

w.Header().Set(corsAllowOriginHeader, origin)
returnOrigin := origin
for _, o := range ch.allowedOrigins {
// A configuration of * is different than explicitly setting an allowed
// origin. Returning arbitrary origin headers an an access control allow
// origin header is unsafe and is not required by any use case.
if o == corsOriginMatchAll {
returnOrigin = "*"
break
}
}
w.Header().Set(corsAllowOriginHeader, returnOrigin)

if r.Method == corsOptionMethod {
return
Expand Down
37 changes: 36 additions & 1 deletion cors_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -327,10 +327,45 @@ func TestCORSHandlerWithCustomValidator(t *testing.T) {
return false
}

CORS(AllowedOriginValidator(originValidator))(testHandler).ServeHTTP(rr, r)
// Specially craft a CORS object.
handleFunc := func(h http.Handler) http.Handler {
c := &cors{
allowedMethods: defaultCorsMethods,
allowedHeaders: defaultCorsHeaders,
allowedOrigins: []string{"http://a.example.com"},
h: h,
}
AllowedOriginValidator(originValidator)(c)
return c
}

handleFunc(testHandler).ServeHTTP(rr, r)
header := rr.HeaderMap.Get(corsAllowOriginHeader)
if header != r.URL.String() {
t.Fatalf("bad header: expected %s to be %s, got %s.", corsAllowOriginHeader, r.URL.String(), header)
}

}

func TestCORSAllowStar(t *testing.T) {
r := newRequest("GET", "http://a.example.com")
r.Header.Set("Origin", r.URL.String())
rr := httptest.NewRecorder()

testHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})
originValidator := func(origin string) bool {
if strings.HasSuffix(origin, ".example.com") {
return true
}
return false
}

CORS(AllowedOriginValidator(originValidator))(testHandler).ServeHTTP(rr, r)
header := rr.HeaderMap.Get(corsAllowOriginHeader)
// Because * is the default CORS policy (which is safe), we should be
// expect a * returned here as the Access Control Allow Origin header
if header != "*" {
t.Fatalf("bad header: expected %s to be %s, got %s.", corsAllowOriginHeader, r.URL.String(), header)
}

}

0 comments on commit 9066371

Please sign in to comment.