hardening

Author: jakecoffman

main.go

@@ -11,7 +11,7 @@ import (
 
 func main() {
 	// find the next git after the first one found
-	os.Args[0] = findGit(os.Getenv("PATH"))
+	os.Args[0] = FindGit(os.Getenv("PATH"))
 	if os.Args[0] == "" {
 		fmt.Fprintln(os.Stderr, "Failed to find git in the path")
 		os.Exit(1)
@@ -39,7 +39,7 @@ func main() {
 	}
 }
 
-func findGit(envPath string) string {
+func FindGit(envPath string) string {
 	paths := strings.Split(envPath, string(os.PathListSeparator))
 	var shimPath string
 
@@ -66,16 +66,26 @@ func findGit(envPath string) string {
 	return ""
 }
 
-var sshUrl = regexp.MustCompile(`^(?P<user>.*?)@(?P<host>.*?):(?:(?P<port>.*?)/)?(?P<path>.*?/.*?)$`)
+var scpUrl = regexp.MustCompile(`^(?P<user>\S+?)@(?P<host>[a-zA-Z\d-]+(\.[a-zA-Z\d-]+)+\.?):(?P<path>.*?/.*?)$`)
 
 func Scrub(argument string) string {
-	u, err := url.Parse(argument)
+	u, err := url.ParseRequestURI(argument)
 	if err == nil && u.Scheme != "" {
 		u.Scheme = "https"
 		return u.String()
 	}
-	if sshUrl.MatchString(argument) {
-		return "https://" + strings.Replace(argument, ":", "/", 1)
+	if scpUrl.MatchString(argument) {
+		newUrl := "https://" + strings.Replace(argument, ":", "/", 1)
+		u, err = url.ParseRequestURI(newUrl)
+		if err != nil {
+			// new URI isn't valid
+			return argument
+		}
+		if u.Host != scpUrl.FindStringSubmatch(argument)[2] {
+			// host changed, possible attack
+			return argument
+		}
+		return newUrl
 	}
 	return argument
 }

main_test.go

@@ -2,6 +2,7 @@ package main
 
 import (
 	"fmt"
+	"net/url"
 	"testing"
 )
 
@@ -43,13 +44,50 @@ func TestScrub(t *testing.T) {
 
 func TestFindGit(t *testing.T) {
 	t.Run("finds the second git", func(t *testing.T) {
-		if v := findGit("test/bin1:test/bin2:test/bin3"); v != "test/bin2/git" {
+		if v := FindGit("test/bin1:test/bin2:test/bin3"); v != "test/bin2/git" {
 			t.Errorf(v)
 		}
 	})
 	t.Run("finds nothing", func(t *testing.T) {
-		if v := findGit("test/bin1"); v != "" {
+		if v := FindGit("test/bin1"); v != "" {
 			t.Errorf(v)
 		}
 	})
 }
+
+func FuzzScrub(f *testing.F) {
+	testcases := []string{
+		"https://github.com/org/repo",
+		"ssh://[email protected]/org/repo",
+		"[email protected]:org/repo",
+		"git://github.com/repo",
+		"HEAD~1",
+		"+/refs/HEAD",
+	}
+	for _, tc := range testcases {
+		f.Add(tc)
+	}
+	f.Fuzz(func(t *testing.T, orig string) {
+		result := Scrub(orig)
+		if result != orig {
+			if extractHost(orig, true) != extractHost(result, false) {
+				// transformed a nonURL into a URL or changed what the URL was, which could be an attack
+				t.Errorf("Before: %q (%q), after: %q (%q)", orig, extractHost(orig, true), result, extractHost(result, false))
+			}
+		}
+	})
+}
+
+func extractHost(input string, orig bool) string {
+	u, err := url.ParseRequestURI(input)
+	if err == nil {
+		return u.Hostname()
+	}
+	if !orig {
+		return ""
+	}
+	if scpUrl.MatchString(input) {
+		return scpUrl.FindStringSubmatch(input)[2]
+	}
+	return ""
+}