webhook: revalidate local hostname before each delivery (#6988)

4 files changed, 19 insertions, 20 deletions

diff --git a/internal/route/api/v1/repo/repo.go b/internal/route/api/v1/repo/repo.go
index 11548ec4..682d2a3b 100644
--- a/internal/route/api/v1/repo/repo.go
+++ b/internal/route/api/v1/repo/repo.go
@@ -248,6 +248,8 @@ func Migrate(c *context.APIContext, f form.MigrateRepo) {
 				c.ErrorStatus(http.StatusUnprocessableEntity, errors.New("You are not allowed to import local repositories."))
 			case addrErr.IsInvalidPath:
 				c.ErrorStatus(http.StatusUnprocessableEntity, errors.New("Invalid local path, it does not exist or not a directory."))
+			case addrErr.IsBlockedLocalAddress:
+				c.ErrorStatus(http.StatusUnprocessableEntity, errors.New("Clone address resolved to a local network address that is implicitly blocked."))
 			default:
 				c.Error(err, "unexpected error")
 			}
diff --git a/internal/route/repo/repo.go b/internal/route/repo/repo.go
index 259aba56..cef25007 100644
--- a/internal/route/repo/repo.go
+++ b/internal/route/repo/repo.go
@@ -180,11 +180,13 @@ func MigratePost(c *context.Context, f form.MigrateRepo) {
 			addrErr := err.(db.ErrInvalidCloneAddr)
 			switch {
 			case addrErr.IsURLError:
-				c.RenderWithErr(c.Tr("form.url_error"), MIGRATE, &f)
+				c.RenderWithErr(c.Tr("repo.migrate.clone_address")+c.Tr("form.url_error"), MIGRATE, &f)
 			case addrErr.IsPermissionDenied:
 				c.RenderWithErr(c.Tr("repo.migrate.permission_denied"), MIGRATE, &f)
 			case addrErr.IsInvalidPath:
 				c.RenderWithErr(c.Tr("repo.migrate.invalid_local_path"), MIGRATE, &f)
+			case addrErr.IsBlockedLocalAddress:
+				c.RenderWithErr(c.Tr("repo.migrate.clone_address_resolved_to_blocked_local_address"), MIGRATE, &f)
 			default:
 				c.Error(err, "unexpected error")
 			}
diff --git a/internal/route/repo/webhook.go b/internal/route/repo/webhook.go
index bed0d0bd..c6ff312a 100644
--- a/internal/route/repo/webhook.go
+++ b/internal/route/repo/webhook.go
@@ -119,20 +119,17 @@ func WebhooksNew(c *context.Context, orCtx *orgRepoContext) {
 	c.Success(orCtx.TmplNew)
 }
 
-func validateWebhook(actor *db.User, l macaron.Locale, w *db.Webhook) (field, msg string, ok bool) {
-	if !actor.IsAdmin {
-		// 🚨 SECURITY: Local addresses must not be allowed by non-admins to prevent SSRF,
-		// see https://github.com/gogs/gogs/issues/5366 for details.
-		payloadURL, err := url.Parse(w.URL)
-		if err != nil {
-			return "PayloadURL", l.Tr("repo.settings.webhook.err_cannot_parse_payload_url", err), false
-		}
-
-		if netutil.IsLocalHostname(payloadURL.Hostname(), conf.Security.LocalNetworkAllowlist) {
-			return "PayloadURL", l.Tr("repo.settings.webhook.err_cannot_use_local_addresses"), false
-		}
+func validateWebhook(l macaron.Locale, w *db.Webhook) (field, msg string, ok bool) {
+	// 🚨 SECURITY: Local addresses must not be allowed by non-admins to prevent SSRF,
+	// see https://github.com/gogs/gogs/issues/5366 for details.
+	payloadURL, err := url.Parse(w.URL)
+	if err != nil {
+		return "PayloadURL", l.Tr("repo.settings.webhook.err_cannot_parse_payload_url", err), false
 	}
 
+	if netutil.IsBlockedLocalHostname(payloadURL.Hostname(), conf.Security.LocalNetworkAllowlist) {
+		return "PayloadURL", l.Tr("repo.settings.webhook.url_resolved_to_blocked_local_address"), false
+	}
 	return "", "", true
 }
 
@@ -144,7 +141,7 @@ func validateAndCreateWebhook(c *context.Context, orCtx *orgRepoContext, w *db.W
 		return
 	}
 
-	field, msg, ok := validateWebhook(c.User, c.Locale, w)
+	field, msg, ok := validateWebhook(c.Locale, w)
 	if !ok {
 		c.FormErr(field)
 		c.RenderWithErr(msg, orCtx.TmplNew, nil)
@@ -348,7 +345,7 @@ func validateAndUpdateWebhook(c *context.Context, orCtx *orgRepoContext, w *db.W
 		return
 	}
 
-	field, msg, ok := validateWebhook(c.User, c.Locale, w)
+	field, msg, ok := validateWebhook(c.Locale, w)
 	if !ok {
 		c.FormErr(field)
 		c.RenderWithErr(msg, orCtx.TmplNew, nil)
diff --git a/internal/route/repo/webhook_test.go b/internal/route/repo/webhook_test.go
index d10a6fcc..784d66ed 100644
--- a/internal/route/repo/webhook_test.go
+++ b/internal/route/repo/webhook_test.go
@@ -31,23 +31,21 @@ func Test_validateWebhook(t *testing.T) {
 	}{
 		{
 			name:    "admin bypass local address check",
-			actor:   &db.User{IsAdmin: true},
-			webhook: &db.Webhook{URL: "http://localhost:3306"},
+			webhook: &db.Webhook{URL: "https://www.google.com"},
 			expOK:   true,
 		},
 
 		{
 			name:     "local address not allowed",
-			actor:    &db.User{},
 			webhook:  &db.Webhook{URL: "http://localhost:3306"},
 			expField: "PayloadURL",
-			expMsg:   "repo.settings.webhook.err_cannot_use_local_addresses",
+			expMsg:   "repo.settings.webhook.url_resolved_to_blocked_local_address",
 			expOK:    false,
 		},
 	}
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			field, msg, ok := validateWebhook(test.actor, l, test.webhook)
+			field, msg, ok := validateWebhook(l, test.webhook)
 			assert.Equal(t, test.expOK, ok)
 			assert.Equal(t, test.expMsg, msg)
 			assert.Equal(t, test.expField, field)