From f6825c1c60a4e125b17f3cd27c26cccb2d520f35 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Guillot?= <f@miniflux.net>
Date: Fri, 25 Mar 2022 22:05:19 -0700
Subject: [PATCH] Fix invalid parsing of data URL

Fetching icons crashes with "slice bounds out of range" error if no encoding is specified.
---
 reader/icon/finder.go      | 57 +++++++++++++++++++++++++++-----------
 reader/icon/finder_test.go | 37 ++++++++++++++++++++++++-
 2 files changed, 77 insertions(+), 17 deletions(-)

diff --git a/reader/icon/finder.go b/reader/icon/finder.go
index e06f6d49..a3985b3f 100644
--- a/reader/icon/finder.go
+++ b/reader/icon/finder.go
@@ -10,6 +10,8 @@ import (
 	"io"
 	"strings"
 
+	stdlib_url "net/url"
+
 	"miniflux.app/config"
 	"miniflux.app/crypto"
 	"miniflux.app/http/client"
@@ -130,40 +132,63 @@ func downloadIcon(iconURL, userAgent string, fetchViaProxy, allowSelfSignedCerti
 	return icon, nil
 }
 
+// https://developer.mozilla.org/en-US/docs/Web/HTTP/Basics_of_HTTP/Data_URIs#syntax
+// data:[<mediatype>][;base64],<data>
 func parseImageDataURL(value string) (*model.Icon, error) {
-	colon := strings.Index(value, ":")
-	semicolon := strings.Index(value, ";")
+	var mediaType string
+	var encoding string
+
+	if !strings.HasPrefix(value, "data:") {
+		return nil, fmt.Errorf(`icon: invalid data URL (missing data:) %q`, value)
+	}
+
+	value = value[5:]
+
 	comma := strings.Index(value, ",")
-
-	if colon <= 0 || semicolon <= 0 || comma <= 0 {
-		return nil, fmt.Errorf(`icon: invalid data url "%s"`, value)
+	if comma < 0 {
+		return nil, fmt.Errorf(`icon: invalid data URL (no comma) %q`, value)
 	}
 
-	mimeType := value[colon+1 : semicolon]
-	encoding := value[semicolon+1 : comma]
 	data := value[comma+1:]
+	semicolon := strings.Index(value[0:comma], ";")
 
-	if encoding != "base64" {
-		return nil, fmt.Errorf(`icon: unsupported data url encoding "%s"`, value)
+	if semicolon > 0 {
+		mediaType = value[0:semicolon]
+		encoding = value[semicolon+1 : comma]
+	} else {
+		mediaType = value[0:comma]
 	}
 
-	if !strings.HasPrefix(mimeType, "image/") {
-		return nil, fmt.Errorf(`icon: invalid mime type "%s"`, mimeType)
+	if !strings.HasPrefix(mediaType, "image/") {
+		return nil, fmt.Errorf(`icon: invalid media type %q`, mediaType)
 	}
 
-	blob, err := base64.StdEncoding.DecodeString(data)
-	if err != nil {
-		return nil, fmt.Errorf(`icon: invalid data "%s" (%v)`, value, err)
+	var blob []byte
+	switch encoding {
+	case "base64":
+		var err error
+		blob, err = base64.StdEncoding.DecodeString(data)
+		if err != nil {
+			return nil, fmt.Errorf(`icon: invalid data %q (%v)`, value, err)
+		}
+	case "":
+		decodedData, err := stdlib_url.QueryUnescape(data)
+		if err != nil {
+			return nil, fmt.Errorf(`icon: unable to decode data URL %q`, value)
+		}
+		blob = []byte(decodedData)
+	default:
+		return nil, fmt.Errorf(`icon: unsupported data URL encoding %q`, value)
 	}
 
 	if len(blob) == 0 {
-		return nil, fmt.Errorf(`icon: empty data "%s"`, value)
+		return nil, fmt.Errorf(`icon: empty data URL %q`, value)
 	}
 
 	icon := &model.Icon{
 		Hash:     crypto.HashFromBytes(blob),
 		Content:  blob,
-		MimeType: mimeType,
+		MimeType: mediaType,
 	}
 
 	return icon, nil
diff --git a/reader/icon/finder_test.go b/reader/icon/finder_test.go
index ee074119..3f32a566 100644
--- a/reader/icon/finder_test.go
+++ b/reader/icon/finder_test.go
@@ -25,6 +25,34 @@ func TestParseImageDataURL(t *testing.T) {
 	}
 }
 
+func TestParseImageDataURLWithNoEncoding(t *testing.T) {
+	iconURL := `data:image/webp,%3Ch1%3EHello%2C%20World%21%3C%2Fh1%3E`
+	icon, err := parseImageDataURL(iconURL)
+	if err != nil {
+		t.Fatalf(`We should be able to parse valid data URL: %v`, err)
+	}
+
+	if icon.MimeType != "image/webp" {
+		t.Fatal(`Invalid mime type parsed`)
+	}
+
+	if string(icon.Content) == "Hello, World!" {
+		t.Fatal(`Value should be URL-decoded`)
+	}
+
+	if icon.Hash == "" {
+		t.Fatal(`Image hash should be computed`)
+	}
+}
+
+func TestParseImageDataURLWithNoMediaTypeAndNoEncoding(t *testing.T) {
+	iconURL := `data:,Hello%2C%20World%21`
+	_, err := parseImageDataURL(iconURL)
+	if err == nil {
+		t.Fatal(`We should detect invalid mime type`)
+	}
+}
+
 func TestParseInvalidImageDataURLWithBadMimeType(t *testing.T) {
 	_, err := parseImageDataURL("data:text/plain;base64,blob")
 	if err == nil {
@@ -39,7 +67,7 @@ func TestParseInvalidImageDataURLWithUnsupportedEncoding(t *testing.T) {
 	}
 }
 
-func TestParseInvalidImageDataURLWithInvalidEncodedData(t *testing.T) {
+func TestParseInvalidImageDataURLWithNoData(t *testing.T) {
 	_, err := parseImageDataURL("data:image/png;base64,")
 	if err == nil {
 		t.Fatal(`We should detect invalid encoded data`)
@@ -53,6 +81,13 @@ func TestParseInvalidImageDataURL(t *testing.T) {
 	}
 }
 
+func TestParseInvalidImageDataURLWithWrongPrefix(t *testing.T) {
+	_, err := parseImageDataURL("data,test")
+	if err == nil {
+		t.Fatal(`We should detect malformed image data URL`)
+	}
+}
+
 func TestParseDocumentWithWhitespaceIconURL(t *testing.T) {
 	html := `<link rel="shortcut icon" href="
 		/static/img/favicon.ico