Browse Source

Fix error message sanitiziation (#3082)

Ethan Koenig 1 year ago
parent
commit
3c1b1ca78e
4 changed files with 64 additions and 31 deletions
  1. 7 26
      models/repo_mirror.go
  2. 48 0
      modules/util/sanitize.go
  3. 3 3
      routers/api/v1/repo/repo.go
  4. 6 2
      routers/repo/repo.go

+ 7 - 26
models/repo_mirror.go

@@ -6,18 +6,18 @@ package models
6 6
 
7 7
 import (
8 8
 	"fmt"
9
-	"strings"
10 9
 	"time"
11 10
 
12
-	"github.com/Unknwon/com"
13
-	"github.com/go-xorm/xorm"
14
-	"gopkg.in/ini.v1"
15
-
16 11
 	"code.gitea.io/git"
17 12
 	"code.gitea.io/gitea/modules/log"
18 13
 	"code.gitea.io/gitea/modules/process"
19 14
 	"code.gitea.io/gitea/modules/setting"
20 15
 	"code.gitea.io/gitea/modules/sync"
16
+	"code.gitea.io/gitea/modules/util"
17
+
18
+	"github.com/Unknwon/com"
19
+	"github.com/go-xorm/xorm"
20
+	"gopkg.in/ini.v1"
21 21
 )
22 22
 
23 23
 // MirrorQueue holds an UniqueQueue object of the mirror
@@ -95,24 +95,6 @@ func (m *Mirror) readAddress() {
95 95
 	}
96 96
 }
97 97
 
98
-// HandleCloneUserCredentials replaces user credentials from HTTP/HTTPS URL
99
-// with placeholder <credentials>.
100
-// It will fail for any other forms of clone addresses.
101
-func HandleCloneUserCredentials(url string, mosaics bool) string {
102
-	i := strings.Index(url, "@")
103
-	if i == -1 {
104
-		return url
105
-	}
106
-	start := strings.Index(url, "://")
107
-	if start == -1 {
108
-		return url
109
-	}
110
-	if mosaics {
111
-		return url[:start+3] + "<credentials>" + url[i:]
112
-	}
113
-	return url[:start+3] + url[i+1:]
114
-}
115
-
116 98
 // sanitizeOutput sanitizes output of a command, replacing occurrences of the
117 99
 // repository's remote address with a sanitized version.
118 100
 func sanitizeOutput(output, repoPath string) (string, error) {
@@ -122,14 +104,13 @@ func sanitizeOutput(output, repoPath string) (string, error) {
122 104
 		// sanitize.
123 105
 		return "", err
124 106
 	}
125
-	sanitized := HandleCloneUserCredentials(remoteAddr, true)
126
-	return strings.Replace(output, remoteAddr, sanitized, -1), nil
107
+	return util.SanitizeMessage(output, remoteAddr), nil
127 108
 }
128 109
 
129 110
 // Address returns mirror address from Git repository config without credentials.
130 111
 func (m *Mirror) Address() string {
131 112
 	m.readAddress()
132
-	return HandleCloneUserCredentials(m.address, false)
113
+	return util.SanitizeURLCredentials(m.address, false)
133 114
 }
134 115
 
135 116
 // FullAddress returns mirror address from Git repository config.

+ 48 - 0
modules/util/sanitize.go

@@ -0,0 +1,48 @@
1
+// Copyright 2017 The Gitea Authors. All rights reserved.
2
+// Use of this source code is governed by a MIT-style
3
+// license that can be found in the LICENSE file.
4
+
5
+package util
6
+
7
+import (
8
+	"net/url"
9
+	"strings"
10
+)
11
+
12
+// urlSafeError wraps an error whose message may contain a sensitive URL
13
+type urlSafeError struct {
14
+	err            error
15
+	unsanitizedURL string
16
+}
17
+
18
+func (err urlSafeError) Error() string {
19
+	return SanitizeMessage(err.err.Error(), err.unsanitizedURL)
20
+}
21
+
22
+// URLSanitizedError returns the sanitized version an error whose message may
23
+// contain a sensitive URL
24
+func URLSanitizedError(err error, unsanitizedURL string) error {
25
+	return urlSafeError{err: err, unsanitizedURL: unsanitizedURL}
26
+}
27
+
28
+// SanitizeMessage sanitizes a message which may contains a sensitive URL
29
+func SanitizeMessage(message, unsanitizedURL string) string {
30
+	sanitizedURL := SanitizeURLCredentials(unsanitizedURL, true)
31
+	return strings.Replace(message, unsanitizedURL, sanitizedURL, -1)
32
+}
33
+
34
+// SanitizeURLCredentials sanitizes a url, either removing user credentials
35
+// or replacing them with a placeholder.
36
+func SanitizeURLCredentials(unsanitizedURL string, usePlaceholder bool) string {
37
+	u, err := url.Parse(unsanitizedURL)
38
+	if err != nil {
39
+		// don't log the error, since it might contain unsanitized URL.
40
+		return "(unparsable url)"
41
+	}
42
+	if u.User != nil && usePlaceholder {
43
+		u.User = url.User("<credentials>")
44
+	} else {
45
+		u.User = nil
46
+	}
47
+	return u.String()
48
+}

+ 3 - 3
routers/api/v1/repo/repo.go

@@ -9,8 +9,6 @@ import (
9 9
 	"net/http"
10 10
 	"strings"
11 11
 
12
-	api "code.gitea.io/sdk/gitea"
13
-
14 12
 	"code.gitea.io/gitea/models"
15 13
 	"code.gitea.io/gitea/modules/auth"
16 14
 	"code.gitea.io/gitea/modules/context"
@@ -18,6 +16,7 @@ import (
18 16
 	"code.gitea.io/gitea/modules/setting"
19 17
 	"code.gitea.io/gitea/modules/util"
20 18
 	"code.gitea.io/gitea/routers/api/v1/convert"
19
+	api "code.gitea.io/sdk/gitea"
21 20
 )
22 21
 
23 22
 // Search repositories via options
@@ -327,12 +326,13 @@ func Migrate(ctx *context.APIContext, form auth.MigrateRepoForm) {
327 326
 		RemoteAddr:  remoteAddr,
328 327
 	})
329 328
 	if err != nil {
329
+		err = util.URLSanitizedError(err, remoteAddr)
330 330
 		if repo != nil {
331 331
 			if errDelete := models.DeleteRepository(ctx.User, ctxUser.ID, repo.ID); errDelete != nil {
332 332
 				log.Error(4, "DeleteRepository: %v", errDelete)
333 333
 			}
334 334
 		}
335
-		ctx.Error(500, "MigrateRepository", models.HandleCloneUserCredentials(err.Error(), true))
335
+		ctx.Error(500, "MigrateRepository", err)
336 336
 		return
337 337
 	}
338 338
 

+ 6 - 2
routers/repo/repo.go

@@ -20,6 +20,7 @@ import (
20 20
 	"code.gitea.io/gitea/modules/context"
21 21
 	"code.gitea.io/gitea/modules/log"
22 22
 	"code.gitea.io/gitea/modules/setting"
23
+	"code.gitea.io/gitea/modules/util"
23 24
 )
24 25
 
25 26
 const (
@@ -232,6 +233,9 @@ func MigratePost(ctx *context.Context, form auth.MigrateRepoForm) {
232 233
 		return
233 234
 	}
234 235
 
236
+	// remoteAddr may contain credentials, so we sanitize it
237
+	err = util.URLSanitizedError(err, remoteAddr)
238
+
235 239
 	if repo != nil {
236 240
 		if errDelete := models.DeleteRepository(ctx.User, ctxUser.ID, repo.ID); errDelete != nil {
237 241
 			log.Error(4, "DeleteRepository: %v", errDelete)
@@ -241,11 +245,11 @@ func MigratePost(ctx *context.Context, form auth.MigrateRepoForm) {
241 245
 	if strings.Contains(err.Error(), "Authentication failed") ||
242 246
 		strings.Contains(err.Error(), "could not read Username") {
243 247
 		ctx.Data["Err_Auth"] = true
244
-		ctx.RenderWithErr(ctx.Tr("form.auth_failed", models.HandleCloneUserCredentials(err.Error(), true)), tplMigrate, &form)
248
+		ctx.RenderWithErr(ctx.Tr("form.auth_failed", err.Error()), tplMigrate, &form)
245 249
 		return
246 250
 	} else if strings.Contains(err.Error(), "fatal:") {
247 251
 		ctx.Data["Err_CloneAddr"] = true
248
-		ctx.RenderWithErr(ctx.Tr("repo.migrate.failed", models.HandleCloneUserCredentials(err.Error(), true)), tplMigrate, &form)
252
+		ctx.RenderWithErr(ctx.Tr("repo.migrate.failed", err.Error()), tplMigrate, &form)
249 253
 		return
250 254
 	}
251 255