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

fix(http): validate redirect URL scheme in HTMLRedirect

Reject any URI that is not a same-origin relative path or an absolute
http(s) URL, preventing attacker-controlled feed entry URLs (e.g.
javascript:, data:, mailto:, scheme-relative //host/...) from being
emitted in a Location header.
Frédéric Guillot 1 месяц назад
Родитель
Сommit
d5e68025d4
2 измененных файлов с 76 добавлено и 1 удалено
  1. 7 1
      internal/http/response/html.go
  2. 69 0
      internal/http/response/html_test.go

+ 7 - 1
internal/http/response/html.go

@@ -4,11 +4,13 @@
 package response // import "miniflux.app/v2/internal/http/response"
 
 import (
+	"fmt"
 	"html"
 	"log/slog"
 	"net/http"
 
 	"miniflux.app/v2/internal/http/request"
+	"miniflux.app/v2/internal/urllib"
 )
 
 // HTML creates a new HTML response with a 200 status code.
@@ -117,8 +119,12 @@ func HTMLNotFound(w http.ResponseWriter, r *http.Request) {
 	builder.Write()
 }
 
-// HTMLRedirect redirects the user to another location.
+// HTMLRedirect redirects the user to a relative path or an absolute http(s) URL.
 func HTMLRedirect(w http.ResponseWriter, r *http.Request, uri string) {
+	if !urllib.IsRelativePath(uri) && !urllib.IsAbsoluteURL(uri) {
+		HTMLBadRequest(w, r, fmt.Errorf("invalid redirect URL: %q", uri))
+		return
+	}
 	http.Redirect(w, r, uri, http.StatusFound)
 }
 

+ 69 - 0
internal/http/response/html_test.go

@@ -183,6 +183,75 @@ func TestHTMLRedirectResponse(t *testing.T) {
 	}
 }
 
+func TestHTMLRedirectAcceptedTargets(t *testing.T) {
+	scenarios := []string{
+		"/feeds",
+		"/category/1/entries",
+		"https://example.org/article",
+		"http://example.org/article",
+	}
+
+	for _, target := range scenarios {
+		t.Run(target, func(t *testing.T) {
+			r, err := http.NewRequest("GET", "/", nil)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			w := httptest.NewRecorder()
+			HTMLRedirect(w, r, target)
+
+			resp := w.Result()
+			defer resp.Body.Close()
+
+			if resp.StatusCode != http.StatusFound {
+				t.Fatalf(`Unexpected status code for %q, got %d instead of %d`, target, resp.StatusCode, http.StatusFound)
+			}
+
+			if actualResult := resp.Header.Get("Location"); actualResult != target {
+				t.Fatalf(`Unexpected redirect location, got %q instead of %q`, actualResult, target)
+			}
+		})
+	}
+}
+
+func TestHTMLRedirectRejectsUnsafeTargets(t *testing.T) {
+	scenarios := []string{
+		"javascript:alert(1)",
+		"JAVASCRIPT:alert(1)",
+		"data:text/html,<script>alert(1)</script>",
+		"vbscript:msgbox(1)",
+		"file:///etc/passwd",
+		"mailto:victim@example.org",
+		"//evil.example.org/path",
+		"ftp://example.org/file",
+		"",
+	}
+
+	for _, target := range scenarios {
+		t.Run(target, func(t *testing.T) {
+			r, err := http.NewRequest("GET", "/", nil)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			w := httptest.NewRecorder()
+			HTMLRedirect(w, r, target)
+
+			resp := w.Result()
+			defer resp.Body.Close()
+
+			if resp.StatusCode != http.StatusBadRequest {
+				t.Fatalf(`Expected 400 for %q, got %d`, target, resp.StatusCode)
+			}
+
+			if location := resp.Header.Get("Location"); location != "" {
+				t.Fatalf(`Expected no Location header for %q, got %q`, target, location)
+			}
+		})
+	}
+}
+
 func TestHTMLRequestedRangeNotSatisfiable(t *testing.T) {
 	r, err := http.NewRequest("GET", "/", nil)
 	if err != nil {