Fix error message sanitiziation (#3082)

4 files changed, 64 insertions, 31 deletions

diff --git a/models/repo_mirror.go b/models/repo_mirror.go
index 92e8788fb5..f52b3eb452 100644
--- a/models/repo_mirror.go
+++ b/models/repo_mirror.go
@@ -6,18 +6,18 @@ package models
 
 import (
 	"fmt"
-	"strings"
 	"time"
 
-	"github.com/Unknwon/com"
-	"github.com/go-xorm/xorm"
-	"gopkg.in/ini.v1"
-
 	"code.gitea.io/git"
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/process"
 	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/sync"
+	"code.gitea.io/gitea/modules/util"
+
+	"github.com/Unknwon/com"
+	"github.com/go-xorm/xorm"
+	"gopkg.in/ini.v1"
 )
 
 // MirrorQueue holds an UniqueQueue object of the mirror
@@ -95,24 +95,6 @@ func (m *Mirror) readAddress() {
 	}
 }
 
-// HandleCloneUserCredentials replaces user credentials from HTTP/HTTPS URL
-// with placeholder <credentials>.
-// It will fail for any other forms of clone addresses.
-func HandleCloneUserCredentials(url string, mosaics bool) string {
-	i := strings.Index(url, "@")
-	if i == -1 {
-		return url
-	}
-	start := strings.Index(url, "://")
-	if start == -1 {
-		return url
-	}
-	if mosaics {
-		return url[:start+3] + "<credentials>" + url[i:]
-	}
-	return url[:start+3] + url[i+1:]
-}
-
 // sanitizeOutput sanitizes output of a command, replacing occurrences of the
 // repository's remote address with a sanitized version.
 func sanitizeOutput(output, repoPath string) (string, error) {
@@ -122,14 +104,13 @@ func sanitizeOutput(output, repoPath string) (string, error) {
 		// sanitize.
 		return "", err
 	}
-	sanitized := HandleCloneUserCredentials(remoteAddr, true)
-	return strings.Replace(output, remoteAddr, sanitized, -1), nil
+	return util.SanitizeMessage(output, remoteAddr), nil
 }
 
 // Address returns mirror address from Git repository config without credentials.
 func (m *Mirror) Address() string {
 	m.readAddress()
-	return HandleCloneUserCredentials(m.address, false)
+	return util.SanitizeURLCredentials(m.address, false)
 }
 
 // FullAddress returns mirror address from Git repository config.
diff --git a/modules/util/sanitize.go b/modules/util/sanitize.go
new file mode 100644
index 0000000000..b1c17b29cf
--- /dev/null
+++ b/modules/util/sanitize.go
@@ -0,0 +1,48 @@
+// Copyright 2017 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 util
+
+import (
+	"net/url"
+	"strings"
+)
+
+// urlSafeError wraps an error whose message may contain a sensitive URL
+type urlSafeError struct {
+	err            error
+	unsanitizedURL string
+}
+
+func (err urlSafeError) Error() string {
+	return SanitizeMessage(err.err.Error(), err.unsanitizedURL)
+}
+
+// URLSanitizedError returns the sanitized version an error whose message may
+// contain a sensitive URL
+func URLSanitizedError(err error, unsanitizedURL string) error {
+	return urlSafeError{err: err, unsanitizedURL: unsanitizedURL}
+}
+
+// SanitizeMessage sanitizes a message which may contains a sensitive URL
+func SanitizeMessage(message, unsanitizedURL string) string {
+	sanitizedURL := SanitizeURLCredentials(unsanitizedURL, true)
+	return strings.Replace(message, unsanitizedURL, sanitizedURL, -1)
+}
+
+// SanitizeURLCredentials sanitizes a url, either removing user credentials
+// or replacing them with a placeholder.
+func SanitizeURLCredentials(unsanitizedURL string, usePlaceholder bool) string {
+	u, err := url.Parse(unsanitizedURL)
+	if err != nil {
+		// don't log the error, since it might contain unsanitized URL.
+		return "(unparsable url)"
+	}
+	if u.User != nil && usePlaceholder {
+		u.User = url.User("<credentials>")
+	} else {
+		u.User = nil
+	}
+	return u.String()
+}
diff --git a/routers/api/v1/repo/repo.go b/routers/api/v1/repo/repo.go
index 158d373416..b154d50a05 100644
--- a/routers/api/v1/repo/repo.go
+++ b/routers/api/v1/repo/repo.go
@@ -9,8 +9,6 @@ import (
 	"net/http"
 	"strings"
 
-	api "code.gitea.io/sdk/gitea"
-
 	"code.gitea.io/gitea/models"
 	"code.gitea.io/gitea/modules/auth"
 	"code.gitea.io/gitea/modules/context"
@@ -18,6 +16,7 @@ import (
 	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/util"
 	"code.gitea.io/gitea/routers/api/v1/convert"
+	api "code.gitea.io/sdk/gitea"
 )
 
 // Search repositories via options
@@ -327,12 +326,13 @@ func Migrate(ctx *context.APIContext, form auth.MigrateRepoForm) {
 		RemoteAddr:  remoteAddr,
 	})
 	if err != nil {
+		err = util.URLSanitizedError(err, remoteAddr)
 		if repo != nil {
 			if errDelete := models.DeleteRepository(ctx.User, ctxUser.ID, repo.ID); errDelete != nil {
 				log.Error(4, "DeleteRepository: %v", errDelete)
 			}
 		}
-		ctx.Error(500, "MigrateRepository", models.HandleCloneUserCredentials(err.Error(), true))
+		ctx.Error(500, "MigrateRepository", err)
 		return
 	}
 
diff --git a/routers/repo/repo.go b/routers/repo/repo.go
index dbe78f6d1e..36105bfe17 100644
--- a/routers/repo/repo.go
+++ b/routers/repo/repo.go
@@ -20,6 +20,7 @@ import (
 	"code.gitea.io/gitea/modules/context"
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/setting"
+	"code.gitea.io/gitea/modules/util"
 )
 
 const (
@@ -232,6 +233,9 @@ func MigratePost(ctx *context.Context, form auth.MigrateRepoForm) {
 		return
 	}
 
+	// remoteAddr may contain credentials, so we sanitize it
+	err = util.URLSanitizedError(err, remoteAddr)
+
 	if repo != nil {
 		if errDelete := models.DeleteRepository(ctx.User, ctxUser.ID, repo.ID); errDelete != nil {
 			log.Error(4, "DeleteRepository: %v", errDelete)
@@ -241,11 +245,11 @@ func MigratePost(ctx *context.Context, form auth.MigrateRepoForm) {
 	if strings.Contains(err.Error(), "Authentication failed") ||
 		strings.Contains(err.Error(), "could not read Username") {
 		ctx.Data["Err_Auth"] = true
-		ctx.RenderWithErr(ctx.Tr("form.auth_failed", models.HandleCloneUserCredentials(err.Error(), true)), tplMigrate, &form)
+		ctx.RenderWithErr(ctx.Tr("form.auth_failed", err.Error()), tplMigrate, &form)
 		return
 	} else if strings.Contains(err.Error(), "fatal:") {
 		ctx.Data["Err_CloneAddr"] = true
-		ctx.RenderWithErr(ctx.Tr("repo.migrate.failed", models.HandleCloneUserCredentials(err.Error(), true)), tplMigrate, &form)
+		ctx.RenderWithErr(ctx.Tr("repo.migrate.failed", err.Error()), tplMigrate, &form)
 		return
 	}