diff --git a/CHANGELOG.md b/CHANGELOG.md index e1149033c..5d9a8f938 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ All notable changes to Gogs are documented in this file. - _Security:_ Denial of service in repository and wiki file listing pages via crafted file names. [#8116](https://github.com/gogs/gogs/pull/8116) - [GHSA-3qq3-668m-v9mj](https://github.com/gogs/gogs/security/advisories/GHSA-3qq3-668m-v9mj) - _Security:_ Reverse proxy authentication header was honored from any remote address, allowing user impersonation when Gogs was reachable directly. The header is now only trusted from addresses listed in `[auth] TRUSTED_PROXY_IPS`. [#8264](https://github.com/gogs/gogs/pull/8264) - [GHSA-w6j9-vw59-27wv](https://github.com/gogs/gogs/security/advisories/GHSA-w6j9-vw59-27wv) +- _Security:_ Server-side request forgery in webhook deliveries via HTTP redirects to local network addresses. [#8263](https://github.com/gogs/gogs/pull/8263) - [GHSA-c4v7-xg93-qf8g](https://github.com/gogs/gogs/security/advisories/GHSA-c4v7-xg93-qf8g) ### Removed diff --git a/internal/database/webhook.go b/internal/database/webhook.go index ccf9b6033..7a0decb83 100644 --- a/internal/database/webhook.go +++ b/internal/database/webhook.go @@ -8,6 +8,7 @@ import ( "encoding/json" "fmt" "io" + "net/http" "net/url" "strings" "time" @@ -711,7 +712,13 @@ func (t *HookTask) deliver() { Header("X-Gogs-Delivery", t.UUID). Header("X-Gogs-Signature", t.Signature). Header("X-Gogs-Event", string(t.EventType)). - SetTLSClientConfig(&tls.Config{InsecureSkipVerify: conf.Webhook.SkipTLSVerify}) + SetTLSClientConfig(&tls.Config{InsecureSkipVerify: conf.Webhook.SkipTLSVerify}). + SetCheckRedirect(func(req *http.Request, _ []*http.Request) error { + // The webhook target is explicitly configured by the user, so any + // redirect would silently retarget the signed payload. Refuse all + // redirects rather than chase them. + return errors.Newf("refusing to follow webhook redirect to %q", req.URL.Redacted()) + }) switch t.ContentType { case JSON: diff --git a/internal/httplib/httplib.go b/internal/httplib/httplib.go index 6b437f271..6cd372ac0 100644 --- a/internal/httplib/httplib.go +++ b/internal/httplib/httplib.go @@ -23,7 +23,11 @@ import ( ) var ( - defaultSetting = Settings{false, "GogsServer", 60 * time.Second, 60 * time.Second, nil, nil, nil, false} + defaultSetting = Settings{ + UserAgent: "GogsServer", + ConnectTimeout: 60 * time.Second, + ReadWriteTimeout: 60 * time.Second, + } defaultCookieJar http.CookieJar settingMutex sync.Mutex ) @@ -95,6 +99,7 @@ type Settings struct { Proxy func(*http.Request) (*url.URL, error) Transport http.RoundTripper EnableCookie bool + CheckRedirect func(req *http.Request, via []*http.Request) error } // Request provides more useful methods for requesting a URL than http.Request. @@ -151,6 +156,13 @@ func (r *Request) SetTLSClientConfig(config *tls.Config) *Request { return r } +// SetCheckRedirect sets the policy invoked by the underlying HTTP client before +// following a redirect. See http.Client.CheckRedirect for semantics. +func (r *Request) SetCheckRedirect(fn func(req *http.Request, via []*http.Request) error) *Request { + r.setting.CheckRedirect = fn + return r +} + // Header add header item string in request. func (r *Request) Header(key, value string) *Request { r.req.Header.Set(key, value) @@ -330,8 +342,9 @@ func (r *Request) getResponse() (*http.Response, error) { } client := &http.Client{ - Transport: trans, - Jar: jar, + Transport: trans, + Jar: jar, + CheckRedirect: r.setting.CheckRedirect, } if len(r.setting.UserAgent) > 0 && r.req.Header.Get("User-Agent") == "" {