Use restricted sanitizer for repository description (#28141)

- Currently the repository description uses the same sanitizer as a
normal markdown document. This means that element such as heading and
images are allowed and can be abused.
- Create a minimal restricted sanitizer for the repository description,
which only allows what the postprocessor currently allows, which are
links and emojis.
- Added unit testing.
- Resolves https://codeberg.org/forgejo/forgejo/issues/1202
- Resolves https://codeberg.org/Codeberg/Community/issues/1122

(cherry picked from commit 631c87cc2347f0036a75dcd21e24429bbca28207)

Co-authored-by: Gusted <postmaster@gusted.xyz>
This commit is contained in:
Earl Warren 2023-11-23 17:34:25 +01:00 committed by GitHub
parent 7d1933717d
commit 1075ff74b5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 56 additions and 5 deletions

View File

@ -584,9 +584,9 @@ func (repo *Repository) DescriptionHTML(ctx context.Context) template.HTML {
}, repo.Description) }, repo.Description)
if err != nil { if err != nil {
log.Error("Failed to render description for %s (ID: %d): %v", repo.Name, repo.ID, err) log.Error("Failed to render description for %s (ID: %d): %v", repo.Name, repo.ID, err)
return template.HTML(markup.Sanitize(repo.Description)) return template.HTML(markup.SanitizeDescription(repo.Description))
} }
return template.HTML(markup.Sanitize(desc)) return template.HTML(markup.SanitizeDescription(desc))
} }
// CloneLink represents different types of clone URLs of repository. // CloneLink represents different types of clone URLs of repository.

View File

@ -19,6 +19,7 @@ import (
// any modification to the underlying policies once it's been created. // any modification to the underlying policies once it's been created.
type Sanitizer struct { type Sanitizer struct {
defaultPolicy *bluemonday.Policy defaultPolicy *bluemonday.Policy
descriptionPolicy *bluemonday.Policy
rendererPolicies map[string]*bluemonday.Policy rendererPolicies map[string]*bluemonday.Policy
init sync.Once init sync.Once
} }
@ -41,6 +42,7 @@ func NewSanitizer() {
func InitializeSanitizer() { func InitializeSanitizer() {
sanitizer.rendererPolicies = map[string]*bluemonday.Policy{} sanitizer.rendererPolicies = map[string]*bluemonday.Policy{}
sanitizer.defaultPolicy = createDefaultPolicy() sanitizer.defaultPolicy = createDefaultPolicy()
sanitizer.descriptionPolicy = createRepoDescriptionPolicy()
for name, renderer := range renderers { for name, renderer := range renderers {
sanitizerRules := renderer.SanitizerRules() sanitizerRules := renderer.SanitizerRules()
@ -161,6 +163,27 @@ func createDefaultPolicy() *bluemonday.Policy {
return policy return policy
} }
// createRepoDescriptionPolicy returns a minimal more strict policy that is used for
// repository descriptions.
func createRepoDescriptionPolicy() *bluemonday.Policy {
policy := bluemonday.NewPolicy()
// Allow italics and bold.
policy.AllowElements("i", "b", "em", "strong")
// Allow code.
policy.AllowElements("code")
// Allow links
policy.AllowAttrs("href", "target", "rel").OnElements("a")
// Allow classes for emojis
policy.AllowAttrs("class").Matching(regexp.MustCompile(`^emoji$`)).OnElements("img", "span")
policy.AllowAttrs("aria-label").OnElements("span")
return policy
}
func addSanitizerRules(policy *bluemonday.Policy, rules []setting.MarkupSanitizerRule) { func addSanitizerRules(policy *bluemonday.Policy, rules []setting.MarkupSanitizerRule) {
for _, rule := range rules { for _, rule := range rules {
if rule.AllowDataURIImages { if rule.AllowDataURIImages {
@ -176,6 +199,12 @@ func addSanitizerRules(policy *bluemonday.Policy, rules []setting.MarkupSanitize
} }
} }
// SanitizeDescription sanitizes the HTML generated for a repository description.
func SanitizeDescription(s string) string {
NewSanitizer()
return sanitizer.descriptionPolicy.Sanitize(s)
}
// Sanitize takes a string that contains a HTML fragment or document and applies policy whitelist. // Sanitize takes a string that contains a HTML fragment or document and applies policy whitelist.
func Sanitize(s string) string { func Sanitize(s string) string {
NewSanitizer() NewSanitizer()

View File

@ -73,6 +73,28 @@ func Test_Sanitizer(t *testing.T) {
} }
} }
func TestDescriptionSanitizer(t *testing.T) {
NewSanitizer()
testCases := []string{
`<h1>Title</h1>`, `Title`,
`<img src='img.png' alt='image'>`, ``,
`<span class="emoji" aria-label="thumbs up">THUMBS UP</span>`, `<span class="emoji" aria-label="thumbs up">THUMBS UP</span>`,
`<span style="color: red">Hello World</span>`, `<span>Hello World</span>`,
`<br>`, ``,
`<a href="https://example.com" target="_blank" rel="noopener noreferrer">https://example.com</a>`, `<a href="https://example.com" target="_blank" rel="noopener noreferrer">https://example.com</a>`,
`<mark>Important!</mark>`, `Important!`,
`<details>Click me! <summary>Nothing to see here.</summary></details>`, `Click me! Nothing to see here.`,
`<input type="hidden">`, ``,
`<b>I</b> have a <i>strong</i> <strong>opinion</strong> about <em>this</em>.`, `<b>I</b> have a <i>strong</i> <strong>opinion</strong> about <em>this</em>.`,
`Provides alternative <code>wg(8)</code> tool`, `Provides alternative <code>wg(8)</code> tool`,
}
for i := 0; i < len(testCases); i += 2 {
assert.Equal(t, testCases[i+1], SanitizeDescription(testCases[i]))
}
}
func TestSanitizeNonEscape(t *testing.T) { func TestSanitizeNonEscape(t *testing.T) {
descStr := "<scrİpt>&lt;script&gt;alert(document.domain)&lt;/script&gt;</scrİpt>" descStr := "<scrİpt>&lt;script&gt;alert(document.domain)&lt;/script&gt;</scrİpt>"