From e762ea7b911bdb792c35441f0b6cf892c510c690 Mon Sep 17 00:00:00 2001
From: Umputun <umputun@gmail.com>
Date: Fri, 26 Mar 2021 13:07:38 -0500
Subject: [PATCH] sanitize both locator.URL and user.Picture

---
 backend/app/store/comment.go      | 18 +++++++++-
 backend/app/store/comment_test.go | 57 +++++++++++++++++++++++++++++++
 2 files changed, 74 insertions(+), 1 deletion(-)

diff --git a/backend/app/store/comment.go b/backend/app/store/comment.go
index ba4e008f..f9ee42a0 100644
--- a/backend/app/store/comment.go
+++ b/backend/app/store/comment.go
@@ -1,6 +1,7 @@
 package store
 
 import (
+	"fmt"
 	"html/template"
 	"regexp"
 	"strings"
@@ -125,7 +126,8 @@ func (c *Comment) Sanitize() {
 	c.Orig = p.Sanitize(c.Orig)
 	c.User.ID = template.HTMLEscapeString(c.User.ID)
 	c.User.Name = c.escapeHTMLWithSome(c.User.Name)
-	c.User.Picture = p.Sanitize(c.User.Picture)
+	c.User.Picture = c.sanitizeAsURL(c.User.Picture)
+	c.Locator.URL = c.sanitizeAsURL(c.Locator.URL)
 }
 
 // Snippet from comment's text
@@ -149,6 +151,20 @@ func (c *Comment) Snippet(limit int) string {
 	return string(snippet) + " ..."
 }
 
+var reHref = regexp.MustCompile(`<a\s+(?:[^>]*?\s+)?href="([^"]*)"`)
+
+// wrap with href to trigger bluemonday sanitizer
+// clean href after sanitizing done
+func (c *Comment) sanitizeAsURL(inp string) string {
+	h := fmt.Sprintf(`<a href="%s">`, inp)
+	clean := bluemonday.UGCPolicy().Sanitize(h)
+	match := reHref.FindStringSubmatch(clean)
+	if len(match) > 1 {
+		return match[1]
+	}
+	return "" // this shouldn't happen as we build the href
+}
+
 func (c *Comment) escapeHTMLWithSome(inp string) string {
 	res := template.HTMLEscapeString(inp)
 	res = strings.Replace(res, "&#34;", "\"", -1)
diff --git a/backend/app/store/comment_test.go b/backend/app/store/comment_test.go
index 2fc413f7..2ffe2ac9 100644
--- a/backend/app/store/comment_test.go
+++ b/backend/app/store/comment_test.go
@@ -14,6 +14,7 @@ func TestComment_Sanitize(t *testing.T) {
 		inp Comment
 		out Comment
 	}{
+
 		{inp: Comment{}, out: Comment{}},
 		{
 			inp: Comment{
@@ -47,6 +48,25 @@ func TestComment_Sanitize(t *testing.T) {
 			inp: Comment{Text: "blah & & 123", User: User{Name: "name <> & ' ` \""}},
 			out: Comment{Text: `blah &amp; &amp; 123`, User: User{Name: "name &lt;&gt; & ' ` \""}},
 		},
+
+		{
+			inp: Comment{Text: "blah blah", Locator: Locator{URL: "javascript:alert('XSS1')"}},
+			out: Comment{Text: "blah blah", Locator: Locator{URL: ""}},
+		},
+		{
+			inp: Comment{Text: "blah blah", Locator: Locator{URL: "javascript:alert(document.domain)//"}},
+			out: Comment{Text: "blah blah", Locator: Locator{URL: ""}},
+		},
+		{
+			inp: Comment{Text: "blah blah", Locator: Locator{URL: "<script>alert()</script>"}},
+			out: Comment{Text: "blah blah", Locator: Locator{URL: "%3Cscript%3Ealert%28%29%3C/script%3E"}},
+		},
+		{
+			inp: Comment{Text: "blah blah",
+				Locator: Locator{URL: "/p/2021/03/23/prep-747/#remark42__comment-1b365913-7056-4920-b9ad-01304bdda085"}},
+			out: Comment{Text: "blah blah",
+				Locator: Locator{URL: "/p/2021/03/23/prep-747/#remark42__comment-1b365913-7056-4920-b9ad-01304bdda085"}},
+		},
 	}
 
 	for n, tt := range tbl {
@@ -159,3 +179,40 @@ func TestComment_Snippet(t *testing.T) {
 		})
 	}
 }
+
+func TestComment_sanitizeAsURL(t *testing.T) {
+
+	tbl := []struct {
+		inp, out string
+	}{
+		{
+			"/p/2021/03/23/prep-747/#remark42__comment-1b365913-7056-4920-b9ad-01304bdda085",
+			"/p/2021/03/23/prep-747/#remark42__comment-1b365913-7056-4920-b9ad-01304bdda085",
+		},
+		{
+			"https://radio-t.com/p/2021/03/23/prep-747/#remark42__comment-1b365913-7056-4920-b9ad-01304bdda085",
+			"https://radio-t.com/p/2021/03/23/prep-747/#remark42__comment-1b365913-7056-4920-b9ad-01304bdda085",
+		},
+		{
+			"javascript:alert(document.domain)//",
+			"",
+		},
+		{
+			"<script>alert()</script>",
+			"%3Cscript%3Ealert%28%29%3C/script%3E",
+		},
+		{
+			"<a href=javascript:alert(document.domain)//>xxx</a>",
+			"",
+		},
+	}
+
+	for i, tt := range tbl {
+		tt := tt
+		c := Comment{}
+		t.Run(strconv.Itoa(i), func(t *testing.T) {
+			assert.Equal(t, tt.out, c.sanitizeAsURL(tt.inp))
+		})
+	}
+
+}