fix(security): only send feed cookies to the feed's website

The cookie configured for a feed was attached to every request made
with its request builder, whatever the host: article pages scraped for
their content, icons hosted on other domains, and well-known feed URLs
probed on the canonical host during discovery. Anyone able to add a
link to the feed, like on a link aggregator, could receive the cookie.

WithCookie now takes the URL the cookie belongs to, and the cookie is
only sent to the same host or to hosts sharing its registrable domain,
so a feed served from rss.example.org still gets the content of its
www.example.org articles. Hosts are converted to lowercase ASCII before
the public suffix lookup, which otherwise treats CO.UK as registrable.

internal/api/subscription_handlers.go

@@ -44,7 +44,7 @@ func (h *handler) discoverSubscriptionsHandler(w http.ResponseWriter, r *http.Re
 		WithCustomApplicationProxyURL(config.Opts.HTTPClientProxyURL()).
 		UseCustomApplicationProxyURL(subscriptionDiscoveryRequest.FetchViaProxy).
 		WithUserAgent(subscriptionDiscoveryRequest.UserAgent, config.Opts.HTTPClientUserAgent()).
-		WithCookie(subscriptionDiscoveryRequest.Cookie).
+		WithCookie(subscriptionDiscoveryRequest.Cookie, subscriptionDiscoveryRequest.URL).
 		WithUsernameAndPassword(subscriptionDiscoveryRequest.Username, subscriptionDiscoveryRequest.Password).
 		IgnoreTLSErrors(subscriptionDiscoveryRequest.AllowSelfSignedCertificates).
 		DisableHTTP2(subscriptionDiscoveryRequest.DisableHTTP2)

internal/reader/fetcher/request_builder.go

@@ -24,6 +24,8 @@ import (
 	"miniflux.app/v2/internal/urllib"
 
 	"golang.org/x/net/http/httpproxy"
+	"golang.org/x/net/idna"
+	"golang.org/x/net/publicsuffix"
 )
 
 const (
@@ -47,6 +49,8 @@ const (
 
 type RequestBuilder struct {
 	headers            http.Header
+	cookie             string
+	cookieSiteHost     string
 	clientProxyURL     *url.URL
 	clientTimeout      time.Duration
 	useClientProxy     bool
@@ -102,9 +106,16 @@ func (r *RequestBuilder) WithUserAgent(userAgent string, defaultUserAgent string
 	return r
 }
 
-func (r *RequestBuilder) WithCookie(cookie string) *RequestBuilder {
+// WithCookie sets the Cookie header for requests to the website of siteURL,
+// that is its host or any host sharing its registrable domain. A cookie
+// configured for a feed must not be sent to the other websites it links to.
+func (r *RequestBuilder) WithCookie(cookie, siteURL string) *RequestBuilder {
 	if cookie != "" {
-		r.headers.Set("Cookie", cookie)
+		r.cookie = cookie
+		r.cookieSiteHost = ""
+		if parsedSiteURL, err := url.Parse(siteURL); err == nil {
+			r.cookieSiteHost = parsedSiteURL.Hostname()
+		}
 	}
 	return r
 }
@@ -295,7 +306,11 @@ func (r *RequestBuilder) ExecuteRequest(requestURL string) (*http.Response, erro
 		return nil, err
 	}
 
-	req.Header = r.headers
+	req.Header = r.headers.Clone()
+	if r.cookie != "" && isSameSite(req.URL.Hostname(), r.cookieSiteHost) {
+		req.Header.Set("Cookie", r.cookie)
+	}
+
 	if r.disableCompression {
 		req.Header.Set("Accept-Encoding", "identity")
 	} else {
@@ -324,6 +339,44 @@ func (r *RequestBuilder) ExecuteRequest(requestURL string) (*http.Response, erro
 	return client.Do(req)
 }
 
+// isSameSite reports whether host and siteHost are the same host, or share the
+// same registrable domain, like "www.example.org" and "feeds.example.org".
+// Hosts under a public suffix, like "alice.github.io" and "bob.github.io", are
+// different websites.
+func isSameSite(host, siteHost string) bool {
+	if host == "" || siteHost == "" {
+		return false
+	}
+
+	if host == siteHost {
+		return true
+	}
+
+	domain, err := registrableDomain(host)
+	if err != nil {
+		return false
+	}
+
+	siteDomain, err := registrableDomain(siteHost)
+	if err != nil {
+		return false
+	}
+
+	return domain == siteDomain
+}
+
+// registrableDomain returns the eTLD+1 of host. The public suffix list only
+// matches lowercase ASCII names, so the host is converted to that form first.
+// IP addresses and single-label hosts don't have one.
+func registrableDomain(host string) (string, error) {
+	asciiHost, err := idna.Lookup.ToASCII(host)
+	if err != nil {
+		return "", err
+	}
+
+	return publicsuffix.EffectiveTLDPlusOne(asciiHost)
+}
+
 func normalizeDialAddress(addr string) string {
 	host, port, err := net.SplitHostPort(addr)
 	if err != nil {

internal/reader/fetcher/request_builder_test.go

@@ -157,7 +157,7 @@ func TestRequestBuilder_WithCookie(t *testing.T) {
 			defer server.Close()
 
 			builder := NewRequestBuilder()
-			resp, err := builder.WithCookie(tt.cookie).ExecuteRequest(server.URL)
+			resp, err := builder.WithCookie(tt.cookie, server.URL).ExecuteRequest(server.URL)
 			if err != nil {
 				t.Fatalf("Expected no error, got %v", err)
 			}
@@ -166,6 +166,145 @@ func TestRequestBuilder_WithCookie(t *testing.T) {
 	}
 }
 
+func TestRequestBuilder_WithCookieOnlySentToSameSite(t *testing.T) {
+	// The proxy receives the requests sent to any host, so no DNS is needed.
+	receivedCookies := make(chan string, 1)
+	proxyServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		select {
+		case receivedCookies <- r.Header.Get("Cookie"):
+		default:
+		}
+		w.WriteHeader(http.StatusOK)
+	}))
+	defer proxyServer.Close()
+
+	proxyURL, err := url.Parse(proxyServer.URL)
+	if err != nil {
+		t.Fatalf("Unable to parse proxy URL: %v", err)
+	}
+
+	// The same builder is used for all requests, like when scraping the entries of a feed.
+	builder := NewRequestBuilder().
+		WithCustomApplicationProxyURL(proxyURL).
+		UseCustomApplicationProxyURL(true).
+		WithCookie("session=secret", "https://feeds.example.org/rss.xml")
+
+	tests := []struct {
+		requestURL string
+		expected   string
+	}{
+		{"http://feeds.example.org/rss.xml", "session=secret"},
+		{"http://www.example.org/article.html", "session=secret"},
+		{"http://example.org/favicon.ico", "session=secret"},
+		{"http://example.net/article.html", ""},
+		{"http://feeds.example.org.example.net/rss.xml", ""},
+		{"http://cdn.example.com/icon.png", ""},
+	}
+
+	for _, tt := range tests {
+		resp, err := builder.ExecuteRequest(tt.requestURL)
+		if err != nil {
+			t.Fatalf("Expected no error for %q, got %v", tt.requestURL, err)
+		}
+		resp.Body.Close()
+
+		select {
+		case cookie := <-receivedCookies:
+			if cookie != tt.expected {
+				t.Errorf("Expected Cookie for %q to be %q, got %q", tt.requestURL, tt.expected, cookie)
+			}
+		default:
+			t.Fatalf("Expected request to %q to be sent through the proxy", tt.requestURL)
+		}
+	}
+}
+
+func TestRequestBuilder_WithCookieNotForwardedOnRedirectToAnotherSite(t *testing.T) {
+	type receivedRequest struct {
+		host   string
+		cookie string
+	}
+
+	receivedRequests := make(chan receivedRequest, 2)
+	proxyServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		select {
+		case receivedRequests <- receivedRequest{r.Host, r.Header.Get("Cookie")}:
+		default:
+		}
+
+		if r.Host == "www.example.org" {
+			http.Redirect(w, r, "http://example.net/article.html", http.StatusFound)
+			return
+		}
+		w.WriteHeader(http.StatusOK)
+	}))
+	defer proxyServer.Close()
+
+	proxyURL, err := url.Parse(proxyServer.URL)
+	if err != nil {
+		t.Fatalf("Unable to parse proxy URL: %v", err)
+	}
+
+	builder := NewRequestBuilder().
+		WithCustomApplicationProxyURL(proxyURL).
+		UseCustomApplicationProxyURL(true).
+		WithCookie("session=secret", "https://feeds.example.org/rss.xml")
+
+	resp, err := builder.ExecuteRequest("http://www.example.org/article.html")
+	if err != nil {
+		t.Fatalf("Expected no error, got %v", err)
+	}
+	resp.Body.Close()
+
+	if len(receivedRequests) != 2 {
+		t.Fatalf("Expected the redirect to be followed, got %d requests", len(receivedRequests))
+	}
+
+	if request := <-receivedRequests; request.cookie != "session=secret" {
+		t.Errorf("Expected Cookie for %q to be %q, got %q", request.host, "session=secret", request.cookie)
+	}
+
+	if request := <-receivedRequests; request.host != "example.net" || request.cookie != "" {
+		t.Errorf("Expected no Cookie to be forwarded to example.net, got %q for %q", request.cookie, request.host)
+	}
+}
+
+func TestIsSameSite(t *testing.T) {
+	tests := []struct {
+		host     string
+		siteHost string
+		expected bool
+	}{
+		{"example.org", "example.org", true},
+		{"www.example.org", "example.org", true},
+		{"example.org", "www.example.org", true},
+		{"www.example.org", "feeds.example.org", true},
+		{"WWW.Example.ORG", "feeds.example.org", true},
+		{"news.example.co.uk", "www.example.co.uk", true},
+		{"xn--bcher-kva.example", "www.bücher.example", true},
+		{"example.net", "example.org", false},
+		{"notexample.org", "example.org", false},
+		{"example.org.example.net", "example.org", false},
+		{"attacker.co.uk", "example.co.uk", false},
+		{"attacker.CO.UK", "www.example.CO.UK", false},
+		{"bob.github.io", "alice.github.io", false},
+		{"127.0.0.1", "127.0.0.1", true},
+		{"10.0.0.1", "192.168.0.1", false},
+		{"::1", "::1", true},
+		{"localhost", "localhost", true},
+		{"intranet", "localhost", false},
+		{"", "example.org", false},
+		{"example.org", "", false},
+		{"", "", false},
+	}
+
+	for _, tt := range tests {
+		if got := isSameSite(tt.host, tt.siteHost); got != tt.expected {
+			t.Errorf("isSameSite(%q, %q) = %v, expected %v", tt.host, tt.siteHost, got, tt.expected)
+		}
+	}
+}
+
 func TestRequestBuilder_WithUsernameAndPassword(t *testing.T) {
 	tests := []struct {
 		name     string
@@ -407,7 +546,7 @@ func TestRequestBuilder_ChainedMethods(t *testing.T) {
 	builder := NewRequestBuilder()
 	resp, err := builder.
 		WithUserAgent("TestAgent/1.0", "DefaultAgent/1.0").
-		WithCookie("test=value").
+		WithCookie("test=value", server.URL).
 		WithETag("etag123").
 		WithTimeout(10 * time.Second).
 		ExecuteRequest(server.URL)

internal/reader/handler/handler.go

@@ -123,7 +123,7 @@ func CreateFeed(store *storage.Storage, userID int64, feedCreationRequest *model
 	requestBuilder := fetcher.NewRequestBuilder().
 		WithUsernameAndPassword(feedCreationRequest.Username, feedCreationRequest.Password).
 		WithUserAgent(feedCreationRequest.UserAgent, config.Opts.HTTPClientUserAgent()).
-		WithCookie(feedCreationRequest.Cookie).
+		WithCookie(feedCreationRequest.Cookie, feedCreationRequest.FeedURL).
 		WithTimeout(config.Opts.HTTPClientTimeout()).
 		WithProxyRotator(proxyrotator.ProxyRotatorInstance).
 		WithCustomFeedProxyURL(feedCreationRequest.ProxyURL).
@@ -232,7 +232,7 @@ func RefreshFeed(store *storage.Storage, userID, feedID int64, forceRefresh bool
 	requestBuilder := fetcher.NewRequestBuilder().
 		WithUsernameAndPassword(originalFeed.Username, originalFeed.Password).
 		WithUserAgent(originalFeed.UserAgent, config.Opts.HTTPClientUserAgent()).
-		WithCookie(originalFeed.Cookie).
+		WithCookie(originalFeed.Cookie, originalFeed.FeedURL).
 		WithTimeout(config.Opts.HTTPClientTimeout()).
 		WithProxyRotator(proxyrotator.ProxyRotatorInstance).
 		WithCustomFeedProxyURL(originalFeed.ProxyURL).

internal/reader/icon/checker.go

@@ -28,7 +28,7 @@ func NewIconChecker(store *storage.Storage, feed *model.Feed) *iconChecker {
 func (c *iconChecker) UpdateOrCreateFeedIcon() {
 	requestBuilder := fetcher.NewRequestBuilder().
 		WithUserAgent(c.feed.UserAgent, config.Opts.HTTPClientUserAgent()).
-		WithCookie(c.feed.Cookie).
+		WithCookie(c.feed.Cookie, c.feed.FeedURL).
 		WithTimeout(config.Opts.HTTPClientTimeout()).
 		WithProxyRotator(proxyrotator.ProxyRotatorInstance).
 		WithCustomFeedProxyURL(c.feed.ProxyURL).

internal/reader/processor/processor.go

@@ -52,7 +52,7 @@ func ProcessFeedEntries(store *storage.Storage, feed *model.Feed, userID int64,
 
 	requestBuilder := fetcher.NewRequestBuilder().
 		WithUserAgent(feed.UserAgent, config.Opts.HTTPClientUserAgent()).
-		WithCookie(feed.Cookie).
+		WithCookie(feed.Cookie, feed.FeedURL).
 		WithTimeout(config.Opts.HTTPClientTimeout()).
 		WithProxyRotator(proxyrotator.ProxyRotatorInstance).
 		WithCustomFeedProxyURL(feed.ProxyURL).
@@ -183,7 +183,7 @@ func ProcessEntryWebPage(feed *model.Feed, entry *model.Entry, user *model.User)
 
 	requestBuilder := fetcher.NewRequestBuilder().
 		WithUserAgent(feed.UserAgent, config.Opts.HTTPClientUserAgent()).
-		WithCookie(feed.Cookie).
+		WithCookie(feed.Cookie, feed.FeedURL).
 		WithTimeout(config.Opts.HTTPClientTimeout()).
 		WithProxyRotator(proxyrotator.ProxyRotatorInstance).
 		WithCustomFeedProxyURL(feed.ProxyURL).

internal/ui/subscription_submit.go

@@ -64,7 +64,7 @@ func (h *handler) submitSubscription(w http.ResponseWriter, r *http.Request) {
 		WithCustomApplicationProxyURL(config.Opts.HTTPClientProxyURL()).
 		UseCustomApplicationProxyURL(subscriptionForm.FetchViaProxy).
 		WithUserAgent(subscriptionForm.UserAgent, config.Opts.HTTPClientUserAgent()).
-		WithCookie(subscriptionForm.Cookie).
+		WithCookie(subscriptionForm.Cookie, subscriptionForm.URL).
 		WithUsernameAndPassword(subscriptionForm.Username, subscriptionForm.Password).
 		IgnoreTLSErrors(subscriptionForm.AllowSelfSignedCertificates).
 		DisableHTTP2(subscriptionForm.DisableHTTP2)