Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use restricted sanitizer for repository description #28141

Merged
merged 2 commits into from
Nov 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions models/repo/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -584,9 +584,9 @@ func (repo *Repository) DescriptionHTML(ctx context.Context) template.HTML {
}, repo.Description)
if err != nil {
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.
Expand Down
35 changes: 32 additions & 3 deletions modules/markup/sanitizer.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,10 @@ import (
// Sanitizer is a protection wrapper of *bluemonday.Policy which does not allow
// any modification to the underlying policies once it's been created.
type Sanitizer struct {
defaultPolicy *bluemonday.Policy
rendererPolicies map[string]*bluemonday.Policy
init sync.Once
defaultPolicy *bluemonday.Policy
descriptionPolicy *bluemonday.Policy
rendererPolicies map[string]*bluemonday.Policy
init sync.Once
}

var (
Expand All @@ -41,6 +42,7 @@ func NewSanitizer() {
func InitializeSanitizer() {
sanitizer.rendererPolicies = map[string]*bluemonday.Policy{}
sanitizer.defaultPolicy = createDefaultPolicy()
sanitizer.descriptionPolicy = createRepoDescriptionPolicy()

for name, renderer := range renderers {
sanitizerRules := renderer.SanitizerRules()
Expand Down Expand Up @@ -161,6 +163,27 @@ func createDefaultPolicy() *bluemonday.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) {
for _, rule := range rules {
if rule.AllowDataURIImages {
Expand All @@ -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.
func Sanitize(s string) string {
NewSanitizer()
Expand Down
22 changes: 22 additions & 0 deletions modules/markup/sanitizer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
descStr := "<scrİpt>&lt;script&gt;alert(document.domain)&lt;/script&gt;</scrİpt>"

Expand Down