Skip to content

Commit 1075ff7

Browse files
earl-warrenGusted
andauthored
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 <[email protected]>
1 parent 7d19337 commit 1075ff7

File tree

3 files changed

+56
-5
lines changed

3 files changed

+56
-5
lines changed

models/repo/repo.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -584,9 +584,9 @@ func (repo *Repository) DescriptionHTML(ctx context.Context) template.HTML {
584584
}, repo.Description)
585585
if err != nil {
586586
log.Error("Failed to render description for %s (ID: %d): %v", repo.Name, repo.ID, err)
587-
return template.HTML(markup.Sanitize(repo.Description))
587+
return template.HTML(markup.SanitizeDescription(repo.Description))
588588
}
589-
return template.HTML(markup.Sanitize(desc))
589+
return template.HTML(markup.SanitizeDescription(desc))
590590
}
591591

592592
// CloneLink represents different types of clone URLs of repository.

modules/markup/sanitizer.go

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,10 @@ import (
1818
// Sanitizer is a protection wrapper of *bluemonday.Policy which does not allow
1919
// any modification to the underlying policies once it's been created.
2020
type Sanitizer struct {
21-
defaultPolicy *bluemonday.Policy
22-
rendererPolicies map[string]*bluemonday.Policy
23-
init sync.Once
21+
defaultPolicy *bluemonday.Policy
22+
descriptionPolicy *bluemonday.Policy
23+
rendererPolicies map[string]*bluemonday.Policy
24+
init sync.Once
2425
}
2526

2627
var (
@@ -41,6 +42,7 @@ func NewSanitizer() {
4142
func InitializeSanitizer() {
4243
sanitizer.rendererPolicies = map[string]*bluemonday.Policy{}
4344
sanitizer.defaultPolicy = createDefaultPolicy()
45+
sanitizer.descriptionPolicy = createRepoDescriptionPolicy()
4446

4547
for name, renderer := range renderers {
4648
sanitizerRules := renderer.SanitizerRules()
@@ -161,6 +163,27 @@ func createDefaultPolicy() *bluemonday.Policy {
161163
return policy
162164
}
163165

166+
// createRepoDescriptionPolicy returns a minimal more strict policy that is used for
167+
// repository descriptions.
168+
func createRepoDescriptionPolicy() *bluemonday.Policy {
169+
policy := bluemonday.NewPolicy()
170+
171+
// Allow italics and bold.
172+
policy.AllowElements("i", "b", "em", "strong")
173+
174+
// Allow code.
175+
policy.AllowElements("code")
176+
177+
// Allow links
178+
policy.AllowAttrs("href", "target", "rel").OnElements("a")
179+
180+
// Allow classes for emojis
181+
policy.AllowAttrs("class").Matching(regexp.MustCompile(`^emoji$`)).OnElements("img", "span")
182+
policy.AllowAttrs("aria-label").OnElements("span")
183+
184+
return policy
185+
}
186+
164187
func addSanitizerRules(policy *bluemonday.Policy, rules []setting.MarkupSanitizerRule) {
165188
for _, rule := range rules {
166189
if rule.AllowDataURIImages {
@@ -176,6 +199,12 @@ func addSanitizerRules(policy *bluemonday.Policy, rules []setting.MarkupSanitize
176199
}
177200
}
178201

202+
// SanitizeDescription sanitizes the HTML generated for a repository description.
203+
func SanitizeDescription(s string) string {
204+
NewSanitizer()
205+
return sanitizer.descriptionPolicy.Sanitize(s)
206+
}
207+
179208
// Sanitize takes a string that contains a HTML fragment or document and applies policy whitelist.
180209
func Sanitize(s string) string {
181210
NewSanitizer()

modules/markup/sanitizer_test.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,28 @@ func Test_Sanitizer(t *testing.T) {
7373
}
7474
}
7575

76+
func TestDescriptionSanitizer(t *testing.T) {
77+
NewSanitizer()
78+
79+
testCases := []string{
80+
`<h1>Title</h1>`, `Title`,
81+
`<img src='img.png' alt='image'>`, ``,
82+
`<span class="emoji" aria-label="thumbs up">THUMBS UP</span>`, `<span class="emoji" aria-label="thumbs up">THUMBS UP</span>`,
83+
`<span style="color: red">Hello World</span>`, `<span>Hello World</span>`,
84+
`<br>`, ``,
85+
`<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>`,
86+
`<mark>Important!</mark>`, `Important!`,
87+
`<details>Click me! <summary>Nothing to see here.</summary></details>`, `Click me! Nothing to see here.`,
88+
`<input type="hidden">`, ``,
89+
`<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>.`,
90+
`Provides alternative <code>wg(8)</code> tool`, `Provides alternative <code>wg(8)</code> tool`,
91+
}
92+
93+
for i := 0; i < len(testCases); i += 2 {
94+
assert.Equal(t, testCases[i+1], SanitizeDescription(testCases[i]))
95+
}
96+
}
97+
7698
func TestSanitizeNonEscape(t *testing.T) {
7799
descStr := "<scrİpt>&lt;script&gt;alert(document.domain)&lt;/script&gt;</scrİpt>"
78100

0 commit comments

Comments
 (0)