From 066515429f1d185431c7d6172d341e3e714d4358 Mon Sep 17 00:00:00 2001 From: Lauris BH Date: Sat, 18 Aug 2018 03:21:20 +0300 Subject: [PATCH] Improve URL validation for external wiki and external issues (#4710) (#4740) * Improve URL validation for external wiki and external issues * Do not allow also localhost address for external URLs --- modules/validation/binding.go | 11 +--- modules/validation/helpers.go | 77 +++++++++++++++++++++++++ modules/validation/helpers_test.go | 90 ++++++++++++++++++++++++++++++ options/locale/locale_en-US.ini | 1 + routers/repo/setting.go | 11 +++- 5 files changed, 180 insertions(+), 10 deletions(-) create mode 100644 modules/validation/helpers.go create mode 100644 modules/validation/helpers_test.go diff --git a/modules/validation/binding.go b/modules/validation/binding.go index 7aaed59c1..bf3e6c4f9 100644 --- a/modules/validation/binding.go +++ b/modules/validation/binding.go @@ -6,7 +6,6 @@ package validation import ( "fmt" - "net/url" "regexp" "strings" @@ -70,13 +69,9 @@ func addValidURLBindingRule() { }, IsValid: func(errs binding.Errors, name string, val interface{}) (bool, binding.Errors) { str := fmt.Sprintf("%v", val) - if len(str) != 0 { - if u, err := url.ParseRequestURI(str); err != nil || - (u.Scheme != "http" && u.Scheme != "https") || - !validPort(portOnly(u.Host)) { - errs.Add([]string{name}, binding.ERR_URL, "Url") - return false, errs - } + if len(str) != 0 && !IsValidURL(str) { + errs.Add([]string{name}, binding.ERR_URL, "Url") + return false, errs } return true, errs diff --git a/modules/validation/helpers.go b/modules/validation/helpers.go new file mode 100644 index 000000000..9a4dfab7a --- /dev/null +++ b/modules/validation/helpers.go @@ -0,0 +1,77 @@ +// Copyright 2018 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package validation + +import ( + "net" + "net/url" + "strings" + + "code.gitea.io/gitea/modules/setting" +) + +var loopbackIPBlocks []*net.IPNet + +func init() { + for _, cidr := range []string{ + "127.0.0.0/8", // IPv4 loopback + "::1/128", // IPv6 loopback + } { + if _, block, err := net.ParseCIDR(cidr); err == nil { + loopbackIPBlocks = append(loopbackIPBlocks, block) + } + } +} + +func isLoopbackIP(ip string) bool { + pip := net.ParseIP(ip) + if pip == nil { + return false + } + for _, block := range loopbackIPBlocks { + if block.Contains(pip) { + return true + } + } + return false +} + +// IsValidURL checks if URL is valid +func IsValidURL(uri string) bool { + if u, err := url.ParseRequestURI(uri); err != nil || + (u.Scheme != "http" && u.Scheme != "https") || + !validPort(portOnly(u.Host)) { + return false + } + + return true +} + +// IsAPIURL checks if URL is current Gitea instance API URL +func IsAPIURL(uri string) bool { + return strings.HasPrefix(strings.ToLower(uri), strings.ToLower(setting.AppURL+"api")) +} + +// IsValidExternalURL checks if URL is valid external URL +func IsValidExternalURL(uri string) bool { + if !IsValidURL(uri) || IsAPIURL(uri) { + return false + } + + u, err := url.ParseRequestURI(uri) + if err != nil { + return false + } + + // Currently check only if not loopback IP is provided to keep compatibility + if isLoopbackIP(u.Hostname()) || strings.ToLower(u.Hostname()) == "localhost" { + return false + } + + // TODO: Later it should be added to allow local network IP addreses + // only if allowed by special setting + + return true +} diff --git a/modules/validation/helpers_test.go b/modules/validation/helpers_test.go new file mode 100644 index 000000000..875625a02 --- /dev/null +++ b/modules/validation/helpers_test.go @@ -0,0 +1,90 @@ +// Copyright 2018 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package validation + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + "code.gitea.io/gitea/modules/setting" +) + +func Test_IsValidURL(t *testing.T) { + cases := []struct { + description string + url string + valid bool + }{ + { + description: "Empty URL", + url: "", + valid: false, + }, + { + description: "Loobpack IPv4 URL", + url: "http://127.0.1.1:5678/", + valid: true, + }, + { + description: "Loobpack IPv6 URL", + url: "https://[::1]/", + valid: true, + }, + { + description: "Missing semicolon after schema", + url: "http//meh/", + valid: false, + }, + } + + for _, testCase := range cases { + t.Run(testCase.description, func(t *testing.T) { + assert.Equal(t, testCase.valid, IsValidURL(testCase.url)) + }) + } +} + +func Test_IsValidExternalURL(t *testing.T) { + setting.AppURL = "https://try.gitea.io/" + + cases := []struct { + description string + url string + valid bool + }{ + { + description: "Current instance URL", + url: "https://try.gitea.io/test", + valid: true, + }, + { + description: "Loobpack IPv4 URL", + url: "http://127.0.1.1:5678/", + valid: false, + }, + { + description: "Current instance API URL", + url: "https://try.gitea.io/api/v1/user/follow", + valid: false, + }, + { + description: "Local network URL", + url: "http://192.168.1.2/api/v1/user/follow", + valid: true, + }, + { + description: "Local URL", + url: "http://LOCALHOST:1234/whatever", + valid: false, + }, + } + + for _, testCase := range cases { + t.Run(testCase.description, func(t *testing.T) { + assert.Equal(t, testCase.valid, IsValidExternalURL(testCase.url)) + }) + } +} diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 3b5847325..e2d4e87d2 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -943,6 +943,7 @@ settings.external_tracker_url = External Issue Tracker URL settings.external_tracker_url_error = The external issue tracker URL is not a valid URL. settings.external_tracker_url_desc = Visitors are redirected to the external issue tracker URL when clicking on the issues tab. settings.tracker_url_format = External Issue Tracker URL Format +settings.tracker_url_format_error = The external issue tracker URL format is not a valid URL. settings.tracker_issue_style = External Issue Tracker Number Format settings.tracker_issue_style.numeric = Numeric settings.tracker_issue_style.alphanumeric = Alphanumeric diff --git a/routers/repo/setting.go b/routers/repo/setting.go index 0d44cb50a..f944179aa 100644 --- a/routers/repo/setting.go +++ b/routers/repo/setting.go @@ -1,4 +1,5 @@ // Copyright 2014 The Gogs Authors. All rights reserved. +// Copyright 2018 The Gitea Authors. All rights reserved. // Use of this source code is governed by a MIT-style // license that can be found in the LICENSE file. @@ -17,6 +18,7 @@ import ( "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/util" + "code.gitea.io/gitea/modules/validation" "code.gitea.io/gitea/routers/utils" ) @@ -157,7 +159,7 @@ func SettingsPost(ctx *context.Context, form auth.RepoSettingForm) { if form.EnableWiki { if form.EnableExternalWiki { - if !strings.HasPrefix(form.ExternalWikiURL, "http://") && !strings.HasPrefix(form.ExternalWikiURL, "https://") { + if !validation.IsValidExternalURL(form.ExternalWikiURL) { ctx.Flash.Error(ctx.Tr("repo.settings.external_wiki_url_error")) ctx.Redirect(repo.Link() + "/settings") return @@ -181,11 +183,16 @@ func SettingsPost(ctx *context.Context, form auth.RepoSettingForm) { if form.EnableIssues { if form.EnableExternalTracker { - if !strings.HasPrefix(form.ExternalTrackerURL, "http://") && !strings.HasPrefix(form.ExternalTrackerURL, "https://") { + if !validation.IsValidExternalURL(form.ExternalTrackerURL) { ctx.Flash.Error(ctx.Tr("repo.settings.external_tracker_url_error")) ctx.Redirect(repo.Link() + "/settings") return } + if len(form.TrackerURLFormat) != 0 && !validation.IsValidExternalURL(form.TrackerURLFormat) { + ctx.Flash.Error(ctx.Tr("repo.settings.tracker_url_format_error")) + ctx.Redirect(repo.Link() + "/settings") + return + } units = append(units, models.RepoUnit{ RepoID: repo.ID, Type: models.UnitTypeExternalTracker,