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

# Conflicts:
#	CHANGELOG.md

CHANGELOG.md

@@ -10,19 +10,23 @@ All notable changes to Gogs are documented in this file.
 - New configuration option `[git.timeout] DIFF` for customizing operation timeout of `git diff`. [#6315](https://github.com/gogs/gogs/issues/6315)
 - New configuration option `[server] SSH_SERVER_MACS` for setting list of accepted MACs for connections to builtin SSH server. [#6434](https://github.com/gogs/gogs/issues/6434)
 - Support specifying custom schema for PostgreSQL. [#6695](https://github.com/gogs/gogs/pull/6695)
+- Support rendering Mermaid diagrams in Markdown. [#6776](https://github.com/gogs/gogs/pull/6776)
 - New languages support: Mongolian. [#6510](https://github.com/gogs/gogs/pull/6510)
 
 ### Changed
 
 - The default branch has been changed to `main`. [#6285](https://github.com/gogs/gogs/pull/6285)
 - MSSQL as database backend is deprecated, installation page no longer shows it as an option. Existing installations and manually craft configuration file continue to work. [#6295](https://github.com/gogs/gogs/pull/6295)
-- Use [Task](https://github.com/go-task/task) as the default build tool for development. [#6297](https://github.com/gogs/gogs/pull/6297)
+- Use [Task](https://github.com/go-task/task) as the build tool. [#6297](https://github.com/gogs/gogs/pull/6297)
+- The required Go version to compile source code changed to 1.16.
 
 ### Fixed
 
-- Add `X-Frame-Options` header to prevent Clickjacking. [#6409](https://github.com/gogs/gogs/issues/6409)
-- _Regression:_ Fixed smart links for issues stops rendering. [#6506](https://github.com/gogs/gogs/issues/6506)
-- _Security:_ Potential SSRF attack by CRLF injection via repository migration. [#6413](https://github.com/gogs/gogs/issues/6413)
+- _Security:_ SSRF in webhook. [#6901](https://github.com/gogs/gogs/issues/6901)
+- _Security:_ XSS in cookies. [#6953](https://github.com/gogs/gogs/issues/6953)
+- _Security:_ OS Command Injection in file uploading. [#6968](https://github.com/gogs/gogs/issues/6968)
+- _Security:_ Remote Command Execution in file editing. [#6555](https://github.com/gogs/gogs/issues/6555)
+- Unable to use LDAP authentication on ARM machines. [#6761](https://github.com/gogs/gogs/issues/6761)
 
 ### Removed
 
@@ -42,6 +46,38 @@ All notable changes to Gogs are documented in this file.
 - Configuration option `[server] LANDING_PAGE` is no longer used, please use `[server] LANDING_URL`.
 - Configuration option `[database] DB_TYPE` is no longer used, please use `[database] TYPE`.
 - Configuration option `[database] PASSWD` is no longer used, please use `[database] PASSWORD`.
+- Remove option to use Makefile as the build tool. [#6980](https://github.com/gogs/gogs/pull/6980)
+
+## 0.12.7
+
+### Fixed
+
+- _Security:_ Stored XSS in issues. [#6919](https://github.com/gogs/gogs/issues/6919)
+- Invalid character in `Access-Control-Allow-Credentials` response header. [#4983](https://github.com/gogs/gogs/issues/4983)
+- Mysterious `ssh: overflow reading version string` errors from builtin SSH server. [#6882](https://github.com/gogs/gogs/issues/6882)
+
+## 0.12.6
+
+### Fixed
+
+- _Security:_ Remote command execution in file uploading. [#6833](https://github.com/gogs/gogs/issues/6833)
+- _Regression:_ Unable to migrate repository from other local Git hosting. Added a new configuration option `[security] LOCAL_NETWORK_ALLOWLIST`, which is a comma separated list of hostnames that are explicitly allowed to be accessed within the local network. [#6841](https://github.com/gogs/gogs/issues/6841)
+- Slow start of Docker containers using NAS devices. [#6554](https://github.com/gogs/gogs/issues/6554)
+
+## 0.12.5
+
+### Fixed
+
+- _Security:_ Potential SSRF in repository migration. [#6754](https://github.com/gogs/gogs/issues/6754)
+- _Security:_ Improper PAM authorization handling. [#6810](https://github.com/gogs/gogs/issues/6810)
+
+## 0.12.4
+
+### Fixed
+
+- _Security:_ Potential SSRF attack by CRLF injection via repository migration. [#6413](https://github.com/gogs/gogs/issues/6413)
+- _Regression:_ Fixed smart links for issues stops rendering. [#6506](https://github.com/gogs/gogs/issues/6506)
+- Added `X-Frame-Options` header to prevent Clickjacking. [#6409](https://github.com/gogs/gogs/issues/6409)
 
 ## 0.12.3
 

conf/locale/locale_en-US.ini

@@ -440,6 +440,7 @@ migrate.clone_address_desc = This can be a HTTP/HTTPS/GIT URL.
 migrate.clone_address_desc_import_local = You're also allowed to migrate a repository by local server path.
 migrate.permission_denied = You are not allowed to import local repositories.
 migrate.invalid_local_path = Invalid local path, it does not exist or not a directory.
+migrate.clone_address_resolved_to_blocked_local_address = Clone address resolved to a local network address that is implicitly blocked.
 migrate.failed = Migration failed: %v
 
 mirror_from = mirror of
@@ -806,7 +807,7 @@ settings.webhook.headers = Headers
 settings.webhook.payload = Payload
 settings.webhook.body = Body
 settings.webhook.err_cannot_parse_payload_url = Cannot parse payload URL: %v
-settings.webhook.err_cannot_use_local_addresses = Non admins are not allowed to use local addresses.
+settings.webhook.url_resolved_to_blocked_local_address = Payload URL resolved to a local network address that is implicitly blocked.
 settings.githooks_desc = Git Hooks are powered by Git itself, you can edit files of supported hooks in the list below to perform custom operations.
 settings.githook_edit_desc = If the hook is inactive, sample content will be presented. Leaving content to an empty value will disable this hook.
 settings.githook_name = Hook Name

internal/db/error.go

@@ -194,9 +194,10 @@ func (err ErrLastOrgOwner) Error() string {
 //         \/     \/|__|              \/                       \/
 
 type ErrInvalidCloneAddr struct {
-	IsURLError         bool
-	IsInvalidPath      bool
-	IsPermissionDenied bool
+	IsURLError            bool
+	IsInvalidPath         bool
+	IsPermissionDenied    bool
+	IsBlockedLocalAddress bool
 }
 
 func IsErrInvalidCloneAddr(err error) bool {
@@ -205,8 +206,8 @@ func IsErrInvalidCloneAddr(err error) bool {
 }
 
 func (err ErrInvalidCloneAddr) Error() string {
-	return fmt.Sprintf("invalid clone address [is_url_error: %v, is_invalid_path: %v, is_permission_denied: %v]",
-		err.IsURLError, err.IsInvalidPath, err.IsPermissionDenied)
+	return fmt.Sprintf("invalid clone address [is_url_error: %v, is_invalid_path: %v, is_permission_denied: %v, is_blocked_local_address: %v]",
+		err.IsURLError, err.IsInvalidPath, err.IsPermissionDenied, err.IsBlockedLocalAddress)
 }
 
 type ErrUpdateTaskNotExist struct {

internal/db/webhook.go

@@ -24,6 +24,7 @@ import (
 	"gogs.io/gogs/internal/conf"
 	"gogs.io/gogs/internal/errutil"
 	"gogs.io/gogs/internal/httplib"
+	"gogs.io/gogs/internal/netutil"
 	"gogs.io/gogs/internal/sync"
 )
 
@@ -688,6 +689,11 @@ func TestWebhook(repo *Repository, event HookEventType, p api.Payloader, webhook
 }
 
 func (t *HookTask) deliver() {
+	if netutil.IsBlockedLocalHostname(t.URL, conf.Security.LocalNetworkAllowlist) {
+		t.ResponseContent = "Payload URL resolved to a local network address that is implicitly blocked."
+		return
+	}
+
 	t.IsDelivered = true
 
 	timeout := time.Duration(conf.Webhook.DeliverTimeout) * time.Second

internal/form/repo.go

@@ -70,8 +70,8 @@ func (f MigrateRepo) ParseRemoteAddr(user *db.User) (string, error) {
 			return "", db.ErrInvalidCloneAddr{IsURLError: true}
 		}
 
-		if netutil.IsLocalHostname(u.Hostname(), conf.Security.LocalNetworkAllowlist) {
-			return "", db.ErrInvalidCloneAddr{IsURLError: true}
+		if netutil.IsBlockedLocalHostname(u.Hostname(), conf.Security.LocalNetworkAllowlist) {
+			return "", db.ErrInvalidCloneAddr{IsBlockedLocalAddress: true}
 		}
 
 		if len(f.AuthUsername)+len(f.AuthPassword) > 0 {

internal/netutil/netutil.go

@@ -47,9 +47,10 @@ func init() {
 	}
 }
 
-// IsLocalHostname returns true if given hostname is resolved to local network
-// address, except exempted from the allowlist.
-func IsLocalHostname(hostname string, allowlist []string) bool {
+// IsBlockedLocalHostname returns true if given hostname is resolved to a local
+// network address that is implicitly blocked (i.e. not exempted from the
+// allowlist).
+func IsBlockedLocalHostname(hostname string, allowlist []string) bool {
 	for _, allow := range allowlist {
 		if hostname == allow {
 			return false

internal/netutil/netutil_test.go

@@ -34,7 +34,7 @@ func TestIsLocalHostname(t *testing.T) {
 	}
 	for _, test := range tests {
 		t.Run("", func(t *testing.T) {
-			assert.Equal(t, test.want, IsLocalHostname(test.hostname, test.allowlist))
+			assert.Equal(t, test.want, IsBlockedLocalHostname(test.hostname, test.allowlist))
 		})
 	}
 }

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")
 			}

internal/route/repo/repo.go

@@ -179,11 +179,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")
 			}

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)

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)