From 154b137b6d97e11c3d52d3b5844d61f8f14d26b9 Mon Sep 17 00:00:00 2001
From: zeripath <art27@cantab.net>
Date: Fri, 28 Feb 2020 20:05:12 +0000
Subject: [PATCH] Relax sanitization as per
 https://github.com/jch/html-pipeline (#10527)

Looking at github/markup#245 it is clear that GH uses https://github.com/jch/html-pipeline to sanitize. This PR relaxes our sanitization to more closely match this.

Fixes #10471
and likely others...
---
 modules/markup/html_test.go | 12 ++++++------
 modules/markup/sanitizer.go | 38 ++++++++++++++++++++++++++++++++++---
 2 files changed, 41 insertions(+), 9 deletions(-)

diff --git a/modules/markup/html_test.go b/modules/markup/html_test.go
index 45145b4025..6dcecc4956 100644
--- a/modules/markup/html_test.go
+++ b/modules/markup/html_test.go
@@ -267,8 +267,8 @@ func TestRender_ShortLinks(t *testing.T) {
 		`<p><a href="`+imgurlWiki+`" rel="nofollow"><img src="`+imgurlWiki+`" title="Link.jpg" alt="Link.jpg"/></a></p>`)
 	test(
 		"[["+favicon+"]]",
-		`<p><a href="`+favicon+`" rel="nofollow"><img src="`+favicon+`" title="favicon.ico"/></a></p>`,
-		`<p><a href="`+favicon+`" rel="nofollow"><img src="`+favicon+`" title="favicon.ico"/></a></p>`)
+		`<p><a href="`+favicon+`" rel="nofollow"><img src="`+favicon+`" title="favicon.ico" alt="`+favicon+`"/></a></p>`,
+		`<p><a href="`+favicon+`" rel="nofollow"><img src="`+favicon+`" title="favicon.ico" alt="`+favicon+`"/></a></p>`)
 	test(
 		"[[Name|Link]]",
 		`<p><a href="`+url+`" rel="nofollow">Name</a></p>`,
@@ -311,16 +311,16 @@ func TestRender_ShortLinks(t *testing.T) {
 		`<p><a href="`+urlWiki+`" rel="nofollow">Link</a> <a href="`+otherURLWiki+`" rel="nofollow">Other Link</a> <a href="`+encodedURLWiki+`" rel="nofollow">Link?</a></p>`)
 	test(
 		"[[Link #.jpg]]",
-		`<p><a href="`+encodedImgurl+`" rel="nofollow"><img src="`+encodedImgurl+`"/></a></p>`,
-		`<p><a href="`+encodedImgurlWiki+`" rel="nofollow"><img src="`+encodedImgurlWiki+`"/></a></p>`)
+		`<p><a href="`+encodedImgurl+`" rel="nofollow"><img src="`+encodedImgurl+`" title="Link #.jpg" alt="Link #.jpg"/></a></p>`,
+		`<p><a href="`+encodedImgurlWiki+`" rel="nofollow"><img src="`+encodedImgurlWiki+`" title="Link #.jpg" alt="Link #.jpg"/></a></p>`)
 	test(
 		"[[Name|Link #.jpg|alt=\"AltName\"|title='Title']]",
 		`<p><a href="`+encodedImgurl+`" rel="nofollow"><img src="`+encodedImgurl+`" title="Title" alt="AltName"/></a></p>`,
 		`<p><a href="`+encodedImgurlWiki+`" rel="nofollow"><img src="`+encodedImgurlWiki+`" title="Title" alt="AltName"/></a></p>`)
 	test(
 		"[[some/path/Link #.jpg]]",
-		`<p><a href="`+notencodedImgurl+`" rel="nofollow"><img src="`+notencodedImgurl+`"/></a></p>`,
-		`<p><a href="`+notencodedImgurlWiki+`" rel="nofollow"><img src="`+notencodedImgurlWiki+`"/></a></p>`)
+		`<p><a href="`+notencodedImgurl+`" rel="nofollow"><img src="`+notencodedImgurl+`" title="Link #.jpg" alt="some/path/Link #.jpg"/></a></p>`,
+		`<p><a href="`+notencodedImgurlWiki+`" rel="nofollow"><img src="`+notencodedImgurlWiki+`" title="Link #.jpg" alt="some/path/Link #.jpg"/></a></p>`)
 	test(
 		"<p><a href=\"https://example.org\">[[foobar]]</a></p>",
 		`<p><a href="https://example.org" rel="nofollow">[[foobar]]</a></p>`,
diff --git a/modules/markup/sanitizer.go b/modules/markup/sanitizer.go
index 49f681f05a..ee9944723e 100644
--- a/modules/markup/sanitizer.go
+++ b/modules/markup/sanitizer.go
@@ -50,12 +50,44 @@ func ReplaceSanitizer() {
 	// Allow keyword markup
 	sanitizer.policy.AllowAttrs("class").Matching(regexp.MustCompile(`^` + keywordClass + `$`)).OnElements("span")
 
-	// Allow <kbd> tags for keyboard shortcut styling
-	sanitizer.policy.AllowElements("kbd")
-
 	// Allow classes for anchors
 	sanitizer.policy.AllowAttrs("class").Matching(regexp.MustCompile(`ref-issue`)).OnElements("a")
 
+	// Allow generally safe attributes
+	generalSafeAttrs := []string{"abbr", "accept", "accept-charset",
+		"accesskey", "action", "align", "alt",
+		"aria-describedby", "aria-hidden", "aria-label", "aria-labelledby",
+		"axis", "border", "cellpadding", "cellspacing", "char",
+		"charoff", "charset", "checked",
+		"clear", "cols", "colspan", "color",
+		"compact", "coords", "datetime", "dir",
+		"disabled", "enctype", "for", "frame",
+		"headers", "height", "hreflang",
+		"hspace", "ismap", "label", "lang",
+		"maxlength", "media", "method",
+		"multiple", "name", "nohref", "noshade",
+		"nowrap", "open", "prompt", "readonly", "rel", "rev",
+		"rows", "rowspan", "rules", "scope",
+		"selected", "shape", "size", "span",
+		"start", "summary", "tabindex", "target",
+		"title", "type", "usemap", "valign", "value",
+		"vspace", "width", "itemprop",
+	}
+
+	generalSafeElements := []string{
+		"h1", "h2", "h3", "h4", "h5", "h6", "h7", "h8", "br", "b", "i", "strong", "em", "a", "pre", "code", "img", "tt",
+		"div", "ins", "del", "sup", "sub", "p", "ol", "ul", "table", "thead", "tbody", "tfoot", "blockquote",
+		"dl", "dt", "dd", "kbd", "q", "samp", "var", "hr", "ruby", "rt", "rp", "li", "tr", "td", "th", "s", "strike", "summary",
+		"details", "caption", "figure", "figcaption",
+		"abbr", "bdo", "cite", "dfn", "mark", "small", "span", "time", "wbr",
+	}
+
+	sanitizer.policy.AllowAttrs(generalSafeAttrs...).OnElements(generalSafeElements...)
+
+	sanitizer.policy.AllowAttrs("itemscope", "itemtype").OnElements("div")
+
+	// FIXME: Need to handle longdesc in img but there is no easy way to do it
+
 	// Custom keyword markup
 	for _, rule := range setting.ExternalSanitizerRules {
 		if rule.Regexp != nil {