Просмотр исходного кода

fix: only relative path should allowed for redirectURL parameter

Protocol-relative URLs like `//example.org` should not be allowed.
Frédéric Guillot 5 месяцев назад
Родитель
Сommit
76df99f3a3
3 измененных файлов с 53 добавлено и 6 удалено
  1. 4 6
      internal/ui/login_check.go
  2. 15 0
      internal/urllib/url.go
  3. 34 0
      internal/urllib/url_test.go

+ 4 - 6
internal/ui/login_check.go

@@ -6,7 +6,6 @@ package ui // import "miniflux.app/v2/internal/ui"
 import (
 	"log/slog"
 	"net/http"
-	"net/url"
 
 	"miniflux.app/v2/internal/config"
 	"miniflux.app/v2/internal/http/cookie"
@@ -17,6 +16,7 @@ import (
 	"miniflux.app/v2/internal/ui/form"
 	"miniflux.app/v2/internal/ui/session"
 	"miniflux.app/v2/internal/ui/view"
+	"miniflux.app/v2/internal/urllib"
 )
 
 func (h *handler) checkLogin(w http.ResponseWriter, r *http.Request) {
@@ -96,11 +96,9 @@ func (h *handler) checkLogin(w http.ResponseWriter, r *http.Request) {
 		config.Opts.BasePath(),
 	))
 
-	if redirectURL != "" {
-		if parsedURL, err := url.Parse(redirectURL); err == nil && !parsedURL.IsAbs() {
-			html.Redirect(w, r, redirectURL)
-			return
-		}
+	if redirectURL != "" && urllib.IsRelativePath(redirectURL) {
+		html.Redirect(w, r, redirectURL)
+		return
 	}
 
 	html.Redirect(w, r, route.Path(h.router, user.DefaultHomePage))

+ 15 - 0
internal/urllib/url.go

@@ -10,6 +10,21 @@ import (
 	"strings"
 )
 
+// IsRelativePath returns true if the link is a relative path.
+func IsRelativePath(link string) bool {
+	if link == "" {
+		return false
+	}
+	if parsedURL, err := url.Parse(link); err == nil {
+		// Only allow relative paths (not scheme-relative URLs like //example.org)
+		// and ensure the URL doesn't have a host component
+		if !parsedURL.IsAbs() && parsedURL.Host == "" && parsedURL.Scheme == "" {
+			return true
+		}
+	}
+	return false
+}
+
 // IsAbsoluteURL returns true if the link is absolute.
 func IsAbsoluteURL(link string) bool {
 	u, err := url.Parse(link)

+ 34 - 0
internal/urllib/url_test.go

@@ -5,6 +5,40 @@ package urllib // import "miniflux.app/v2/internal/urllib"
 
 import "testing"
 
+func TestIsRelativePath(t *testing.T) {
+	scenarios := map[string]bool{
+		// Valid relative paths
+		"path/to/file.ext":    true,
+		"./path/to/file.ext":  true,
+		"../path/to/file.ext": true,
+		"file.ext":            true,
+		"./file.ext":          true,
+		"../file.ext":         true,
+		"/absolute/path":      true,
+		"path?query=value":    true,
+		"path#fragment":       true,
+		"path?query#fragment": true,
+
+		// Not relative paths
+		"https://example.org/file.ext": false,
+		"http://example.org/file.ext":  false,
+		"//example.org/file.ext":       false,
+		"//example.org":                false,
+		"ftp://example.org/file.ext":   false,
+		"mailto:user@example.org":      false,
+		"magnet:?xt=urn:btih:example":  false,
+		"":                             false,
+		"magnet:?xt.1=urn:sha1:YNCKHTQCWBTRNJIV4WNAE52SJUQCZO5C": false,
+	}
+
+	for input, expected := range scenarios {
+		actual := IsRelativePath(input)
+		if actual != expected {
+			t.Errorf(`Unexpected result for IsRelativePath, got %v instead of %v for %q`, actual, expected, input)
+		}
+	}
+}
+
 func TestIsAbsoluteURL(t *testing.T) {
 	scenarios := map[string]bool{
 		"https://example.org/file.pdf": true,