Subject: [PATCH links] ensure redirect url's are relative

From: Peter Sanchez <[email-masked]>
Date: Tue, 18 Aug 2026 14:04:40 -0600

Changelog-added: security check to ensure redirect urls are always
  relative
---
 admin/routes.go     | 12 ++--------
 core/routes.go      | 23 +++++++-----------
 core/routes_test.go | 57 ++++++++++++++++++++++++++++++++++++++++++++-
 helpers.go          | 10 ++++++++
 helpers_test.go     | 27 +++++++++++++++++++++
 5 files changed, 103 insertions(+), 26 deletions(-)

diff --git a/admin/routes.go b/admin/routes.go
index 2f7e7a7..26c904e 100644
--- a/admin/routes.go
+++ b/admin/routes.go
@@ -2,7 +2,6 @@ package admin
 
 import (
 	"fmt"
-	"html/template"
 	"links"
 	"links/internal/localizer"
 	"links/models"
@@ -1020,19 +1019,12 @@ func (s *Service) BillingList(c echo.Context) error {
 		"filterSelector":   filterSelector,
 	}
 
-	queryParams := c.QueryParams()
 	if historyResult.Payments.PageInfo.HasPrevPage {
-		tmpQueryParams := queryParams
-		tmpQueryParams.Del("next")
-		tmpQueryParams.Set("prev", historyResult.Payments.PageInfo.Cursor)
-		gmap["prevURL"] = template.URL(tmpQueryParams.Encode())
+		gmap["prevURL"] = links.GetPaginationParams(c, "prev", historyResult.Payments.PageInfo.Cursor, "next")
 	}
 
 	if historyResult.Payments.PageInfo.HasNextPage {
-		tmpQueryParams := queryParams
-		tmpQueryParams.Del("prev")
-		tmpQueryParams.Set("next", historyResult.Payments.PageInfo.Cursor)
-		gmap["nextURL"] = template.URL(tmpQueryParams.Encode())
+		gmap["nextURL"] = links.GetPaginationParams(c, "next", historyResult.Payments.PageInfo.Cursor, "prev")
 	}
 	return s.Render(c, http.StatusOK, "admin_billing_list.html", gmap)
 }
diff --git a/core/routes.go b/core/routes.go
index 2b2f835..2c51149 100644
--- a/core/routes.go
+++ b/core/routes.go
@@ -2716,10 +2716,8 @@ func (s *Service) OrgLinkDelete(c echo.Context) error {
 		}
 
 		messages.Success(c, lt.Translate("Bookmark successfully deleted"))
-		redirect := c.QueryParam("next")
-		if redirect == "" {
-			redirect = c.Echo().Reverse(s.RouteName("home_link_list"))
-		}
+		redirect := links.SafeRedirect(c.QueryParam("next"),
+			c.Echo().Reverse(s.RouteName("home_link_list")))
 		return c.Redirect(http.StatusMovedPermanently, redirect)
 	}
 
@@ -2760,7 +2758,7 @@ func (s *Service) OrgLinkDelete(c echo.Context) error {
 
 	deleteURL := c.Echo().Reverse(s.RouteName("link_delete"), link.Hash)
 	backURL := c.Echo().Reverse(s.RouteName("home_link_list"))
-	if next := c.QueryParam("next"); next != "" {
+	if next := links.SafeRedirect(c.QueryParam("next"), ""); next != "" {
 		deleteURL = deleteURL + "?next=" + url.QueryEscape(next)
 		backURL = next
 	}
@@ -3075,7 +3073,7 @@ func (s *Service) QRManageDelete(c echo.Context) error {
 	if err != nil {
 		return echo.NotFoundHandler(c)
 	}
-	next := c.QueryParam("next")
+	next := links.SafeRedirect(c.QueryParam("next"), "")
 	if next == "" {
 		return echo.NotFoundHandler(c)
 	}
@@ -3441,7 +3439,7 @@ func (s *Service) OrgLinkStarToggle(c echo.Context) error {
 	if err != nil {
 		return err
 	}
-	redirect := c.QueryParam("next")
+	redirect := links.SafeRedirect(c.QueryParam("next"), "")
 	if redirect == "" {
 		redirect = c.Request().Header.Get("Referer")
 	}
@@ -3491,7 +3489,7 @@ func (s *Service) OrgLinkAsReadToggle(c echo.Context) error {
 	if err != nil {
 		return err
 	}
-	redirect := c.QueryParam("next")
+	redirect := links.SafeRedirect(c.QueryParam("next"), "")
 	if redirect == "" {
 		redirect = c.Request().Header.Get("Referer")
 	}
@@ -3600,13 +3598,8 @@ func (s *Service) FollowToggle(c echo.Context) error {
 		return err
 	}
 
-	var nextURL string
-	if c.QueryParam("next") != "" {
-		nextURL = c.QueryParam("next")
-	} else {
-		nextURL = c.Echo().Reverse("core:org_link_list", orgSlug)
-
-	}
+	nextURL := links.SafeRedirect(c.QueryParam("next"),
+		c.Echo().Reverse("core:org_link_list", orgSlug))
 	return c.Redirect(http.StatusMovedPermanently, nextURL)
 }
 
diff --git a/core/routes_test.go b/core/routes_test.go
index 0c8b320..6b7bc75 100644
--- a/core/routes_test.go
+++ b/core/routes_test.go
@@ -599,7 +599,7 @@ func TestHandlers(t *testing.T) {
 		httpmock.RegisterResponder("POST", "http://127.0.0.1:8080/query", jsonResponse)
 
 		f := make(url.Values)
-		f.Set("next", "http//redirect.com")
+		f.Set("next", "/qr")
 		request := httptest.NewRequest(http.MethodPost, "/?"+f.Encode(), nil)
 		recorder := httptest.NewRecorder()
 		ctx := &server.Context{
@@ -613,6 +613,61 @@ func TestHandlers(t *testing.T) {
 		err = test.MakeRequestWithDomain(srv, coreService.QRManageDelete, ctx, domains[0])
 		c.NoError(err)
 		c.Equal(http.StatusMovedPermanently, recorder.Code)
+		c.Equal("/qr", recorder.Header().Get("Location"))
+	})
+
+	// QRManageDelete treats an unusable ?next= as a missing one and 404s rather
+	// than handing an off-site URL to c.Redirect.
+	t.Run("qr delete rejects off-site next", func(t *testing.T) {
+		for _, hostile := range []string{"https://evil.com", "//evil.com", "http//redirect.com"} {
+			f := make(url.Values)
+			f.Set("next", hostile)
+			request := httptest.NewRequest(http.MethodPost, "/?"+f.Encode(), nil)
+			recorder := httptest.NewRecorder()
+			ctx := &server.Context{
+				Server:  srv,
+				Context: e.NewContext(request, recorder),
+				User:    loggedInUser,
+			}
+			ctx.SetPath("/qr/:id/delete")
+			ctx.SetParamNames("id")
+			ctx.SetParamValues("100")
+			err := test.MakeRequestWithDomain(srv, coreService.QRManageDelete, ctx, domains[0])
+
+			var httpErr *echo.HTTPError
+			c.ErrorAs(err, &httpErr, "hostile next %q must not redirect", hostile)
+			c.Equal(http.StatusNotFound, httpErr.Code)
+		}
+	})
+
+	// The toggle/follow handlers fall back to an in-app route instead of 404ing.
+	t.Run("follow toggle ignores off-site next", func(t *testing.T) {
+		httpmock.Activate()
+		defer httpmock.DeactivateAndReset()
+		jsonResponse, err := httpmock.NewJsonResponder(http.StatusOK, map[string]any{
+			"data": map[string]any{"follow": map[string]any{"success": true, "message": ""}},
+		})
+		c.NoError(err)
+		httpmock.RegisterResponder("POST", "http://127.0.0.1:8080/query", jsonResponse)
+
+		f := make(url.Values)
+		f.Set("next", "https://evil.com")
+		request := httptest.NewRequest(http.MethodGet, "/?"+f.Encode(), nil)
+		recorder := httptest.NewRecorder()
+		ctx := &server.Context{
+			Server:  srv,
+			Context: e.NewContext(request, recorder),
+			User:    loggedInUser,
+		}
+		ctx.SetPath("/:slug/follow-toggle/:action")
+		ctx.SetParamNames("slug", "action")
+		ctx.SetParamValues("test-org", "follow")
+		err = test.MakeRequestWithDomain(srv, coreService.FollowToggle, ctx, domains[0])
+		c.NoError(err)
+
+		location := recorder.Header().Get("Location")
+		c.NotEqual("https://evil.com", location)
+		c.True(strings.HasPrefix(location, "/"), "expected in-app path, got %q", location)
 	})
 
 	t.Run("user feed rss with valid hash", func(t *testing.T) {
diff --git a/helpers.go b/helpers.go
index ad2a855..ceaf027 100644
--- a/helpers.go
+++ b/helpers.go
@@ -519,6 +519,16 @@ func SetCursorVars(c echo.Context, op *gqlclient.Operation) bool {
 	return false
 }
 
+func SafeRedirect(next, fallback string) string {
+	if next == "" || next[0] != '/' {
+		return fallback
+	}
+	if len(next) > 1 && (next[1] == '/' || next[1] == '\\') {
+		return fallback
+	}
+	return next
+}
+
 // GetPaginationParams returns the params needed for cursor pagination
 func GetPaginationParams(c echo.Context, pagvar, cursor string, exclude ...string) template.URL {
 	q := make(url.Values)
diff --git a/helpers_test.go b/helpers_test.go
index 0e44115..2cf5938 100644
--- a/helpers_test.go
+++ b/helpers_test.go
@@ -90,3 +90,30 @@ func TestSetCursorVars(t *testing.T) {
 		})
 	}
 }
+
+func TestSafeRedirect(t *testing.T) {
+	const fallback = "/home"
+
+	tests := []struct {
+		name string
+		next string
+		want string
+	}{
+		{"empty falls back", "", fallback},
+		{"relative path", "/org/links", "/org/links"},
+		{"root", "/", "/"},
+		{"relative with query", "/org/links?tag=go", "/org/links?tag=go"},
+		{"absolute https", "https://evil.com", fallback},
+		{"absolute http", "http://evil.com/path", fallback},
+		{"scheme relative", "//evil.com", fallback},
+		{"backslash scheme relative", "/\\evil.com", fallback},
+		{"schemeless host", "evil.com", fallback},
+		{"javascript uri", "javascript:alert(1)", fallback},
+		{"missing colon host", "http//redirect.com", fallback},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			require.Equal(t, tt.want, SafeRedirect(tt.next, fallback))
+		})
+	}
+}
-- 
2.54.0