Improve URL validation for external wiki and external issues (#4710)

* Improve URL validation for external wiki and external issues * Do not allow also localhost address for external URLs
go-gitea · Aug 15, 2018 · 9246612 · 9246612
1 parent 0449330
commit 9246612
Show file tree

Hide file tree

Showing 5 changed files with 180 additions and 10 deletions.
diff --git 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
@@ -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
@@ -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
@@ -987,6 +987,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
@@ -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,