فهرست منبع

feat(icon): disallow fetching icon on private networks

This change avoid possible SSRF issues and it's configurable at the instance level
Frédéric Guillot 10 ماه پیش
والد
کامیت
29f6dc8896
5فایلهای تغییر یافته به همراه96 افزوده شده و 0 حذف شده
  1. 9 0
      internal/config/options.go
  2. 24 0
      internal/config/options_parsing_test.go
  3. 40 0
      internal/reader/icon/finder.go
  4. 18 0
      internal/reader/icon/finder_test.go
  5. 5 0
      miniflux.1

+ 9 - 0
internal/config/options.go

@@ -300,6 +300,11 @@ func NewConfigOptions() *configOptions {
 				rawValue:        "0",
 				valueType:       boolType,
 			},
+			"ICON_FETCH_ALLOW_PRIVATE_NETWORKS": {
+				parsedBoolValue: false,
+				rawValue:        "0",
+				valueType:       boolType,
+			},
 			"INVIDIOUS_INSTANCE": {
 				parsedStringValue: "yewtu.be",
 				rawValue:          "yewtu.be",
@@ -783,6 +788,10 @@ func (c *configOptions) HTTPS() bool {
 	return c.options["HTTPS"].parsedBoolValue
 }
 
+func (c *configOptions) IconFetchAllowPrivateNetworks() bool {
+	return c.options["ICON_FETCH_ALLOW_PRIVATE_NETWORKS"].parsedBoolValue
+}
+
 func (c *configOptions) InvidiousInstance() string {
 	return c.options["INVIDIOUS_INSTANCE"].parsedStringValue
 }

+ 24 - 0
internal/config/options_parsing_test.go

@@ -1364,6 +1364,30 @@ func TestHTTPClientTimeoutOptionParsing(t *testing.T) {
 	}
 }
 
+func TestIconFetchAllowPrivateNetworksOptionParsing(t *testing.T) {
+	configParser := NewConfigParser()
+
+	if configParser.options.IconFetchAllowPrivateNetworks() {
+		t.Fatalf("Expected ICON_FETCH_ALLOW_PRIVATE_NETWORKS to be disabled by default")
+	}
+
+	if err := configParser.parseLines([]string{"ICON_FETCH_ALLOW_PRIVATE_NETWORKS=1"}); err != nil {
+		t.Fatalf("Unexpected error: %v", err)
+	}
+
+	if !configParser.options.IconFetchAllowPrivateNetworks() {
+		t.Fatalf("Expected ICON_FETCH_ALLOW_PRIVATE_NETWORKS to be enabled")
+	}
+
+	if err := configParser.parseLines([]string{"ICON_FETCH_ALLOW_PRIVATE_NETWORKS=0"}); err != nil {
+		t.Fatalf("Unexpected error: %v", err)
+	}
+
+	if configParser.options.IconFetchAllowPrivateNetworks() {
+		t.Fatalf("Expected ICON_FETCH_ALLOW_PRIVATE_NETWORKS to be disabled")
+	}
+}
+
 func TestHTTPServerTimeoutOptionParsing(t *testing.T) {
 	configParser := NewConfigParser()
 

+ 40 - 0
internal/reader/icon/finder.go

@@ -156,6 +156,10 @@ func (f *iconFinder) downloadIcon(iconURL string) (*model.Icon, error) {
 		slog.String("icon_url", iconURL),
 	)
 
+	if err := ensureRemoteIconURLAllowed(iconURL, config.Opts.IconFetchAllowPrivateNetworks()); err != nil {
+		return nil, err
+	}
+
 	responseHandler := fetcher.NewResponseHandler(f.requestBuilder.ExecuteRequest(iconURL))
 	defer responseHandler.Close()
 
@@ -330,3 +334,39 @@ func parseImageDataURL(value string) (*model.Icon, error) {
 		MimeType: mediaType,
 	}, nil
 }
+
+func ensureRemoteIconURLAllowed(iconURL string, allowPrivateNetworks bool) error {
+	parsedURL, err := url.Parse(iconURL)
+	if err != nil {
+		return fmt.Errorf("icon: invalid icon URL %q: %w", iconURL, err)
+	}
+
+	if !parsedURL.IsAbs() {
+		return fmt.Errorf("icon: icon URL %q must be absolute", iconURL)
+	}
+
+	scheme := strings.ToLower(parsedURL.Scheme)
+	if scheme != "http" && scheme != "https" {
+		return fmt.Errorf("icon: unsupported icon URL scheme %q", parsedURL.Scheme)
+	}
+
+	hostname := parsedURL.Hostname()
+	if hostname == "" {
+		return fmt.Errorf("icon: icon URL %q has no hostname", iconURL)
+	}
+
+	if allowPrivateNetworks {
+		return nil
+	}
+
+	isPrivate, err := urllib.ResolvesToPrivateIP(hostname)
+	if err != nil {
+		return fmt.Errorf("icon: unable to resolve icon hostname %q: %w", hostname, err)
+	}
+
+	if isPrivate {
+		return fmt.Errorf("icon: refusing to download icon from private network host %q", hostname)
+	}
+
+	return nil
+}

+ 18 - 0
internal/reader/icon/finder_test.go

@@ -400,6 +400,24 @@ func TestResizeIconWebp(t *testing.T) {
 	}
 }
 
+func TestEnsureRemoteIconURLAllowedRejectsPrivateNetworks(t *testing.T) {
+	if err := ensureRemoteIconURLAllowed("http://192.168.0.1/favicon.ico", false); err == nil {
+		t.Fatal("Expected private network hosts to be rejected")
+	}
+}
+
+func TestEnsureRemoteIconURLAllowedAllowsPublicNetworks(t *testing.T) {
+	if err := ensureRemoteIconURLAllowed("https://1.1.1.1/favicon.ico", false); err != nil {
+		t.Fatalf("Expected public network hosts to be allowed: %v", err)
+	}
+}
+
+func TestEnsureRemoteIconURLAllowedAllowsPrivateWhenEnabled(t *testing.T) {
+	if err := ensureRemoteIconURLAllowed("http://10.0.0.5/icon.png", true); err != nil {
+		t.Fatalf("Expected private network hosts to be allowed when explicitly enabled: %v", err)
+	}
+}
+
 func TestResizeInvalidImage(t *testing.T) {
 	icon := model.Icon{
 		Content:  []byte("invalid data"),

+ 5 - 0
miniflux.1

@@ -336,6 +336,11 @@ Forces cookies to use secure flag and send HSTS header\&.
 .br
 Default is disabled\&.
 .TP
+.B ICON_FETCH_ALLOW_PRIVATE_NETWORKS
+Set to 1 to allow downloading favicons that resolve to private or loopback networks\&.
+.br
+Disabled by default, private networks are refused\&.
+.TP
 .B INVIDIOUS_INSTANCE
 Set a custom invidious instance to use\&.
 .br