From 01b5ed2e8fc20be1f9c0dd285d1c814f0efa954a Mon Sep 17 00:00:00 2001 From: Benjamin Foote Date: Tue, 5 May 2020 11:02:04 -0700 Subject: [PATCH 1/3] case insensitive, no javascript --- handlers/login.go | 4 ++-- handlers/login_test.go | 3 +++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/handlers/login.go b/handlers/login.go index afb85a82..cabc218e 100644 --- a/handlers/login.go +++ b/handlers/login.go @@ -94,7 +94,7 @@ func getValidRequestedURL(r *http.Request) (string, error) { if urlparam == "" { return "", errNoURL } - if !strings.HasPrefix(urlparam, "http://") && !strings.HasPrefix(urlparam, "https://") { + if !strings.HasPrefix(strings.ToLower(urlparam), "http://") && !strings.HasPrefix(strings.ToLower(urlparam), "https://") { return "", errURLNotHTTP } u, err := url.Parse(urlparam) @@ -111,7 +111,7 @@ func getValidRequestedURL(r *http.Request) (string, error) { // log.Debugf("validateRequestedURL %s:%s", k, v) for _, vval := range v { for _, bad := range badStrings { - if strings.HasPrefix(vval, bad) { + if strings.HasPrefix(strings.ToLower(vval), bad) { return "", fmt.Errorf("%w looks bad: %s includes %s", errDangerQS, vval, bad) } } diff --git a/handlers/login_test.go b/handlers/login_test.go index 9d51c054..89d2b121 100644 --- a/handlers/login_test.go +++ b/handlers/login_test.go @@ -17,7 +17,10 @@ func Test_getValidRequestedURL(t *testing.T) { }{ {"no https", "example.com/dest", "", true}, {"redirection chaining", "http://example.com/dest?url=https://", "", true}, + {"redirection chaining upper case", "http://example.com/dest?url=HTTPS://someplaceelse.com", "", true}, + {"redirection chaining no protocol", "http://example.com/dest?url=//someplaceelse.com", "", true}, {"data uri", "http://example.com/dest?url=data:text/plain,Example+Text", "", true}, + {"javascript uri", "http://example.com/dest?url=javascript:alert(1)", "", true}, {"not in domain", "http://somewherelse.com/", "", true}, {"should warn", "https://example.com/", "https://example.com/", false}, {"should be fine", "http://example.com/", "http://example.com/", false}, From fe766d268a819358e6f4965968ad737e8467502f Mon Sep 17 00:00:00 2001 From: Benjamin Foote Date: Tue, 5 May 2020 11:23:41 -0700 Subject: [PATCH 2/3] test callback url check --- pkg/cfg/oauth_test.go | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) create mode 100644 pkg/cfg/oauth_test.go diff --git a/pkg/cfg/oauth_test.go b/pkg/cfg/oauth_test.go new file mode 100644 index 00000000..f86b07ec --- /dev/null +++ b/pkg/cfg/oauth_test.go @@ -0,0 +1,42 @@ +/* + +Copyright 2020 The Vouch Proxy Authors. +Use of this source code is governed by The MIT License (MIT) that +can be found in the LICENSE file. Software distributed under The +MIT License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES +OR CONDITIONS OF ANY KIND, either express or implied. + +*/ + +package cfg + +import ( + "os" + "path/filepath" + "testing" +) + +func setUp(configFile string) { + os.Setenv("VOUCH_CONFIG", filepath.Join(os.Getenv("VOUCH_ROOT"), configFile)) + InitForTestPurposes() +} + +func Test_checkCallbackConfig(t *testing.T) { + setUp("/config/testing/handler_login_url.yml") + + tests := []struct { + name string + url string + wantErr bool + }{ + {"correct", "http://vouch.example.com:9090/auth", false}, + {"bad", "http://vouch.notgonna.com:9090/somewhereelse", true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if err := checkCallbackConfig(tt.url); (err != nil) != tt.wantErr { + t.Errorf("checkCallbackConfig() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} From 7fd21bb9559377ee5256506a91e5dc4cc08e75c7 Mon Sep 17 00:00:00 2001 From: Benjamin Foote Date: Tue, 5 May 2020 11:32:06 -0700 Subject: [PATCH 3/3] no protocol-less URLs --- handlers/login.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/handlers/login.go b/handlers/login.go index cabc218e..fe0d3628 100644 --- a/handlers/login.go +++ b/handlers/login.go @@ -85,7 +85,7 @@ var ( errInvalidURL = errors.New("requested destination URL appears to be invalid") errURLNotHTTP = errors.New("requested destination URL is not a valid URL (does not begin with 'http://' or 'https://')") errDangerQS = errors.New("requested destination URL has a dangerous query string") - badStrings = []string{"http://", "https://", "data:", "ftp://", "ftps://"} + badStrings = []string{"http://", "https://", "data:", "ftp://", "ftps://", "//", "javascript:"} ) func getValidRequestedURL(r *http.Request) (string, error) {