Skip to content

Commit

Permalink
Merge branch 'master' into feature/license
Browse files Browse the repository at this point in the history
  • Loading branch information
bnfinet committed May 5, 2020
2 parents 566dbd0 + 7fd21bb commit 0765273
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 3 deletions.
6 changes: 3 additions & 3 deletions handlers/login.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,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) {
Expand All @@ -104,7 +104,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)
Expand All @@ -121,7 +121,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)
}
}
Expand Down
3 changes: 3 additions & 0 deletions handlers/login_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,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},
Expand Down
42 changes: 42 additions & 0 deletions pkg/cfg/oauth_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
})
}
}

0 comments on commit 0765273

Please sign in to comment.