From 4fe0c0d7e35ddf84bcf6c0ce7e240382ad14306c Mon Sep 17 00:00:00 2001 From: martinpasaribu Date: Sun, 6 Oct 2024 17:21:19 +0700 Subject: [PATCH 1/2] improve MultipartForm test --- context_test.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/context_test.go b/context_test.go index d363ed790..1fd89edb4 100644 --- a/context_test.go +++ b/context_test.go @@ -774,6 +774,11 @@ func TestContextMultipartForm(t *testing.T) { buf := new(bytes.Buffer) mw := multipart.NewWriter(buf) mw.WriteField("name", "Jon Snow") + fileContent := "This is a test file" + w, err := mw.CreateFormFile("file", "test.txt") + if assert.NoError(t, err) { + w.Write([]byte(fileContent)) + } mw.Close() req := httptest.NewRequest(http.MethodPost, "/", buf) req.Header.Set(HeaderContentType, mw.FormDataContentType()) @@ -782,6 +787,13 @@ func TestContextMultipartForm(t *testing.T) { f, err := c.MultipartForm() if assert.NoError(t, err) { assert.NotNil(t, f) + + files := f.File["file"] + if assert.Len(t, files, 1) { + file := files[0] + assert.Equal(t, "test.txt", file.Filename) + assert.Equal(t, int64(len(fileContent)), file.Size) + } } } From 20677a0637a91795e292802c4011d446d3c60eb7 Mon Sep 17 00:00:00 2001 From: martinyonatann Date: Wed, 11 Nov 2026 11:16:10 +0700 Subject: [PATCH 2/2] fix(middleware): prevent open redirect (CWE-601) in Redirect middlewares --- middleware/redirect.go | 55 +++++++++++++++--- middleware/redirect_test.go | 111 ++++++++++++++++++++++++++++++++++++ 2 files changed, 158 insertions(+), 8 deletions(-) diff --git a/middleware/redirect.go b/middleware/redirect.go index bb7045cfe..a8ee0e976 100644 --- a/middleware/redirect.go +++ b/middleware/redirect.go @@ -134,8 +134,22 @@ func (config RedirectConfig) ToMiddleware() (echo.MiddlewareFunc, error) { } req, scheme := c.Request(), c.Scheme() + switch scheme { + case "ws": + scheme = "http" + case "wss": + scheme = "https" + } host := req.Host - if ok, url := config.redirect(scheme, host, req.RequestURI); ok { + if !isValidRedirectHost(host) { + return next(c) + } + uri := req.RequestURI + if uri == "" || uri[0] != '/' { + uri = req.URL.RequestURI() + } + uri = sanitizeURI(uri) + if ok, url := config.redirect(scheme, host, uri); ok { return c.Redirect(config.Code, url) } @@ -144,6 +158,31 @@ func (config RedirectConfig) ToMiddleware() (echo.MiddlewareFunc, error) { }, nil } +func hasWWWPrefix(host string) bool { + return len(host) >= len(www) && strings.EqualFold(host[:len(www)], www) +} + +func stripWWWPrefix(host string) string { + if hasWWWPrefix(host) { + return host[len(www):] + } + return host +} + +func isValidRedirectHost(host string) bool { + host = stripWWWPrefix(host) + if host == "" || host[0] == ':' || host[0] == '.' { + return false + } + for i := 0; i < len(host); i++ { + ch := host[i] + if ch <= 0x20 || ch >= 0x7f || strings.IndexByte(`/\@?#`, ch) != -1 { + return false + } + } + return true +} + var redirectHTTPS = func(scheme, host, uri string) (bool, string) { if scheme != "https" { return true, "https://" + host + uri @@ -153,8 +192,8 @@ var redirectHTTPS = func(scheme, host, uri string) (bool, string) { var redirectHTTPSWWW = func(scheme, host, uri string) (bool, string) { // Redirect if not HTTPS OR missing www prefix (needs either fix) - if scheme != "https" || !strings.HasPrefix(host, www) { - host = strings.TrimPrefix(host, www) // Remove www if present to avoid duplication + if scheme != "https" || !hasWWWPrefix(host) { + host = stripWWWPrefix(host) // Remove www if present to avoid duplication return true, "https://www." + host + uri } return false, "" @@ -162,23 +201,23 @@ var redirectHTTPSWWW = func(scheme, host, uri string) (bool, string) { var redirectNonHTTPSWWW = func(scheme, host, uri string) (ok bool, url string) { // Redirect if not HTTPS OR has www prefix (needs either fix) - if scheme != "https" || strings.HasPrefix(host, www) { - host = strings.TrimPrefix(host, www) + if scheme != "https" || hasWWWPrefix(host) { + host = stripWWWPrefix(host) return true, "https://" + host + uri } return false, "" } var redirectWWW = func(scheme, host, uri string) (bool, string) { - if !strings.HasPrefix(host, www) { + if !hasWWWPrefix(host) { return true, scheme + "://www." + host + uri } return false, "" } var redirectNonWWW = func(scheme, host, uri string) (bool, string) { - if strings.HasPrefix(host, www) { - return true, scheme + "://" + host[4:] + uri + if hasWWWPrefix(host) { + return true, scheme + "://" + host[len(www):] + uri } return false, "" } diff --git a/middleware/redirect_test.go b/middleware/redirect_test.go index 2d9d6528f..bb7de0319 100644 --- a/middleware/redirect_test.go +++ b/middleware/redirect_test.go @@ -336,3 +336,114 @@ func TestRedirectHTTPSRedirect_forwardedProtoTrust(t *testing.T) { }) } } + +func TestRedirectSecurityHardening(t *testing.T) { + var testCases = []struct { + name string + middleware middlewareGenerator + whenHost string + whenRequestURI string + whenHeader http.Header + expectLocation string + expectStatusCode int + }{ + { + name: "NonWWWRedirect ignores bare www. host to prevent https:///attacker.com open redirect", + middleware: NonWWWRedirect, + whenHost: "www.", + whenRequestURI: "/attacker.com/login", + expectLocation: "", + expectStatusCode: http.StatusOK, + }, + { + name: "HTTPSNonWWWRedirect ignores bare www. host to prevent https:///attacker.com open redirect", + middleware: HTTPSNonWWWRedirect, + whenHost: "www.", + whenRequestURI: "/attacker.com/login", + expectLocation: "", + expectStatusCode: http.StatusOK, + }, + { + name: "HTTPSRedirect ignores empty host", + middleware: HTTPSRedirect, + whenHost: "", + whenRequestURI: "/attacker.com", + expectLocation: "", + expectStatusCode: http.StatusOK, + }, + { + name: "HTTPSRedirect ignores host with authority or path delimiters", + middleware: HTTPSRedirect, + whenHost: "attacker.com@labstack.com", + whenRequestURI: "/", + expectLocation: "", + expectStatusCode: http.StatusOK, + }, + { + name: "NonWWWRedirect handles uppercase WWW. prefix", + middleware: NonWWWRedirect, + whenHost: "WWW.labstack.com", + whenRequestURI: "/docs", + expectLocation: "http://labstack.com/docs", + expectStatusCode: http.StatusMovedPermanently, + }, + { + name: "HTTPSWWWRedirect does not duplicate uppercase WWW. prefix", + middleware: HTTPSWWWRedirect, + whenHost: "WWW.labstack.com", + whenRequestURI: "/docs", + expectLocation: "https://www.labstack.com/docs", + expectStatusCode: http.StatusMovedPermanently, + }, + { + name: "HTTPSRedirect sanitizes double-slash and control chars in RequestURI", + middleware: HTTPSRedirect, + whenHost: "labstack.com", + whenRequestURI: "//attacker.com/phish", + expectLocation: "https://labstack.com/attacker.com/phish", + expectStatusCode: http.StatusMovedPermanently, + }, + { + name: "WWWRedirect normalizes ws scheme from X-Forwarded-Proto to http", + middleware: WWWRedirect, + whenHost: "labstack.com", + whenRequestURI: "/ws", + whenHeader: http.Header{echo.HeaderXForwardedProto: {"ws"}}, + expectLocation: "http://www.labstack.com/ws", + expectStatusCode: http.StatusMovedPermanently, + }, + { + name: "HTTPSRedirect treats wss scheme from X-Forwarded-Proto as https", + middleware: HTTPSRedirect, + whenHost: "labstack.com", + whenRequestURI: "/ws", + whenHeader: http.Header{echo.HeaderXForwardedProto: {"wss"}}, + expectLocation: "", + expectStatusCode: http.StatusOK, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + e := echo.New() + req := httptest.NewRequest(http.MethodGet, "/", nil) + req.Host = tc.whenHost + req.RequestURI = tc.whenRequestURI + req.RemoteAddr = "10.0.0.1:1234" + if tc.whenHeader != nil { + req.Header = tc.whenHeader + } + res := httptest.NewRecorder() + c := e.NewContext(req, res) + + err := tc.middleware()(func(c *echo.Context) error { + return c.NoContent(http.StatusOK) + })(c) + + assert.NoError(t, err) + assert.Equal(t, tc.expectStatusCode, res.Code) + assert.Equal(t, tc.expectLocation, res.Header().Get(echo.HeaderLocation)) + }) + } +} +