Skip to content

Commit 88cc3ad

Browse files
zeripathzeripath
committed
Add Ambiguous Character Detection to Description, FullName and others
Users can change their description, full-names and others to include ambiguous and invisible characters which may lead to misleading information. This PR adds ambiguous/invisible character detection to these fields. Signed-off-by: Andrew Thornton <[email protected]>
1 parent 7258a12 commit 88cc3ad

36 files changed

+247
-144
lines changed

‎modules/charset/escape.go

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -22,18 +22,12 @@ const RuneNBSP = 0xa0
2222
// EscapeControlHTML escapes the unicode control sequences in a provided html document
2323
func EscapeControlHTML(text string, locale translation.Locale, allowed ...rune) (escaped *EscapeStatus, output string) {
2424
sb := &strings.Builder{}
25-
outputStream := &HTMLStreamerWriter{Writer: sb}
26-
streamer := NewEscapeStreamer(locale, outputStream, allowed...).(*escapeStreamer)
27-
28-
if err := StreamHTML(strings.NewReader(text), streamer); err != nil {
29-
streamer.escaped.HasError = true
30-
log.Error("Error whilst escaping: %v", err)
31-
}
32-
return streamer.escaped, sb.String()
25+
escaped, _ = EscapeControlHTMLReader(strings.NewReader(text), sb, locale, allowed...)
26+
return escaped, sb.String()
3327
}
3428

35-
// EscapeControlReaders escapes the unicode control sequences in a provider reader and writer in a locale and returns the findings as an EscapeStatus and the escaped []byte
36-
func EscapeControlReader(reader io.Reader, writer io.Writer, locale translation.Locale, allowed ...rune) (escaped *EscapeStatus, err error) {
29+
// EscapeControlHTMLReader escapes the unicode control sequences in a provided reader of an HTML document and writer in a locale and returns the findings as an EscapeStatus
30+
func EscapeControlHTMLReader(reader io.Reader, writer io.Writer, locale translation.Locale, allowed ...rune) (escaped *EscapeStatus, err error) {
3731
outputStream := &HTMLStreamerWriter{Writer: writer}
3832
streamer := NewEscapeStreamer(locale, outputStream, allowed...).(*escapeStreamer)
3933

@@ -56,3 +50,15 @@ func EscapeControlString(text string, locale translation.Locale, allowed ...rune
5650
}
5751
return streamer.escaped, sb.String()
5852
}
53+
54+
// EscapeControlStringWriter escapes the unicode control sequences in a string and provided writer in a locale and returns the findings as an EscapeStatus
55+
func EscapeControlStringWriter(text string, writer io.Writer, locale translation.Locale, allowed ...rune) (escaped *EscapeStatus, err error) {
56+
outputStream := &HTMLStreamerWriter{Writer: writer}
57+
streamer := NewEscapeStreamer(locale, outputStream, allowed...).(*escapeStreamer)
58+
59+
if err = streamer.Text(text); err != nil {
60+
streamer.escaped.HasError = true
61+
log.Error("Error whilst escaping: %v", err)
62+
}
63+
return streamer.escaped, err
64+
}

‎modules/charset/escape_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ func TestEscapeControlReader(t *testing.T) {
173173
t.Run(tt.name, func(t *testing.T) {
174174
input := strings.NewReader(tt.text)
175175
output := &strings.Builder{}
176-
status, err := EscapeControlReader(input, output, translation.NewLocale("en_US"))
176+
status, err := EscapeControlHTMLReader(input, output, translation.NewLocale("en_US"))
177177
result := output.String()
178178
if err != nil {
179179
t.Errorf("EscapeControlReader(): err = %v", err)

‎modules/templates/helper.go

Lines changed: 87 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import (
3131
repo_model "code.gitea.io/gitea/models/repo"
3232
user_model "code.gitea.io/gitea/models/user"
3333
"code.gitea.io/gitea/modules/base"
34+
"code.gitea.io/gitea/modules/charset"
3435
"code.gitea.io/gitea/modules/emoji"
3536
"code.gitea.io/gitea/modules/git"
3637
giturl "code.gitea.io/gitea/modules/git/url"
@@ -42,6 +43,7 @@ import (
4243
"code.gitea.io/gitea/modules/setting"
4344
"code.gitea.io/gitea/modules/svg"
4445
"code.gitea.io/gitea/modules/timeutil"
46+
"code.gitea.io/gitea/modules/translation"
4547
"code.gitea.io/gitea/modules/util"
4648
"code.gitea.io/gitea/services/gitdiff"
4749

@@ -343,12 +345,15 @@ func NewFuncMap() []template.FuncMap {
343345
}
344346
return false
345347
},
346-
"svg": SVG,
347-
"avatar": Avatar,
348-
"avatarHTML": AvatarHTML,
349-
"avatarByAction": AvatarByAction,
350-
"avatarByEmail": AvatarByEmail,
351-
"repoAvatar": RepoAvatar,
348+
"svg": SVG,
349+
"avatar": Avatar,
350+
"avatarHTML": AvatarHTML,
351+
"avatarByAction": AvatarByAction,
352+
"avatarByEmail": AvatarByEmail,
353+
"escapeAmbiguous": EscapeAmbiguous,
354+
"escapeAmbiguousHTML": EscapeAmbiguousHTML,
355+
"escapeAmbiguousLink": EscapeAmbiguousLink,
356+
"repoAvatar": RepoAvatar,
352357
"SortArrow": func(normSort, revSort, urlSort string, isDefault bool) template.HTML {
353358
// if needed
354359
if len(normSort) == 0 || len(urlSort) == 0 {
@@ -680,6 +685,67 @@ func AvatarByEmail(email, name string, others ...interface{}) template.HTML {
680685
return template.HTML("")
681686
}
682687

688+
// EscapeAmbiguous
689+
func EscapeAmbiguous(locale translation.Locale, text string) template.HTML {
690+
sb := &strings.Builder{}
691+
status, _ := charset.EscapeControlStringWriter(text, sb, locale)
692+
escapeStatusSwitch(locale, sb, status)
693+
694+
return template.HTML(sb.String())
695+
}
696+
697+
// EscapeAmbiguousHTML
698+
func EscapeAmbiguousHTML(locale translation.Locale, html string) template.HTML {
699+
sb := &strings.Builder{}
700+
status, _ := charset.EscapeControlHTMLReader(strings.NewReader(html), sb, locale)
701+
escapeStatusSwitch(locale, sb, status)
702+
return template.HTML(sb.String())
703+
}
704+
705+
// EscapeAmbiguousLink takes a locale, text body - which is assumed to be a string not html, href and other attributes
706+
func EscapeAmbiguousLink(locale translation.Locale, text, href string, attrs ...string) template.HTML {
707+
sb := &strings.Builder{}
708+
_, _ = sb.WriteString(`<a href="`)
709+
template.HTMLEscape(sb, []byte(href))
710+
_, _ = sb.WriteString(`"`)
711+
attrValue := false
712+
for _, attr := range attrs {
713+
if attrValue {
714+
_, _ = sb.WriteString(`="`)
715+
} else {
716+
_, _ = sb.WriteString(` `)
717+
}
718+
template.HTMLEscape(sb, []byte(attr))
719+
if attrValue {
720+
_, _ = sb.WriteString(`"`)
721+
}
722+
attrValue = !attrValue
723+
}
724+
_, _ = sb.WriteString(`>`)
725+
status, _ := charset.EscapeControlStringWriter(text, sb, locale)
726+
_, _ = sb.WriteString(`</a>`)
727+
728+
escapeStatusSwitch(locale, sb, status)
729+
return template.HTML(sb.String())
730+
}
731+
732+
func escapeStatusSwitch(locale translation.Locale, sb *strings.Builder, status *charset.EscapeStatus) {
733+
if status.Escaped {
734+
_, _ = sb.WriteString(`<a href="" class="toggle-escape-button" title="`)
735+
if status.HasInvisible {
736+
_, _ = sb.WriteString(locale.Tr("invisible_runes"))
737+
}
738+
if status.HasInvisible && status.HasAmbiguous {
739+
_, _ = sb.WriteString(` `)
740+
}
741+
if status.HasAmbiguous {
742+
_, _ = sb.WriteString(locale.Tr("ambiguous_runes"))
743+
}
744+
745+
_, _ = sb.WriteString(`"></a>`)
746+
}
747+
}
748+
683749
// Safe render raw as HTML
684750
func Safe(raw string) template.HTML {
685751
return template.HTML(raw)
@@ -711,13 +777,13 @@ func DotEscape(raw string) string {
711777
}
712778

713779
// RenderCommitMessage renders commit message with XSS-safe and special links.
714-
func RenderCommitMessage(ctx context.Context, msg, urlPrefix string, metas map[string]string) template.HTML {
715-
return RenderCommitMessageLink(ctx, msg, urlPrefix, "", metas)
780+
func RenderCommitMessage(ctx context.Context, locale translation.Locale, msg, urlPrefix string, metas map[string]string) template.HTML {
781+
return RenderCommitMessageLink(ctx, locale, msg, urlPrefix, "", metas)
716782
}
717783

718784
// RenderCommitMessageLink renders commit message as a XXS-safe link to the provided
719785
// default url, handling for special links.
720-
func RenderCommitMessageLink(ctx context.Context, msg, urlPrefix, urlDefault string, metas map[string]string) template.HTML {
786+
func RenderCommitMessageLink(ctx context.Context, locale translation.Locale, msg, urlPrefix, urlDefault string, metas map[string]string) template.HTML {
721787
cleanMsg := template.HTMLEscapeString(msg)
722788
// we can safely assume that it will not return any error, since there
723789
// shouldn't be any special HTML.
@@ -731,16 +797,17 @@ func RenderCommitMessageLink(ctx context.Context, msg, urlPrefix, urlDefault str
731797
log.Error("RenderCommitMessage: %v", err)
732798
return ""
733799
}
734-
msgLines := strings.Split(strings.TrimSpace(fullMessage), "\n")
800+
msgLines := strings.SplitN(strings.TrimSpace(fullMessage), "\n", 2)
735801
if len(msgLines) == 0 {
736802
return template.HTML("")
737803
}
738-
return template.HTML(msgLines[0])
804+
_, renderedMessage := charset.EscapeControlHTML(msgLines[0], locale)
805+
return template.HTML(renderedMessage)
739806
}
740807

741808
// RenderCommitMessageLinkSubject renders commit message as a XXS-safe link to
742809
// the provided default url, handling for special links without email to links.
743-
func RenderCommitMessageLinkSubject(ctx context.Context, msg, urlPrefix, urlDefault string, metas map[string]string) template.HTML {
810+
func RenderCommitMessageLinkSubject(ctx context.Context, locale translation.Locale, msg, urlPrefix, urlDefault string, metas map[string]string) template.HTML {
744811
msgLine := strings.TrimLeftFunc(msg, unicode.IsSpace)
745812
lineEnd := strings.IndexByte(msgLine, '\n')
746813
if lineEnd > 0 {
@@ -763,11 +830,12 @@ func RenderCommitMessageLinkSubject(ctx context.Context, msg, urlPrefix, urlDefa
763830
log.Error("RenderCommitMessageSubject: %v", err)
764831
return template.HTML("")
765832
}
833+
_, renderedMessage = charset.EscapeControlHTML(renderedMessage, locale)
766834
return template.HTML(renderedMessage)
767835
}
768836

769837
// RenderCommitBody extracts the body of a commit message without its title.
770-
func RenderCommitBody(ctx context.Context, msg, urlPrefix string, metas map[string]string) template.HTML {
838+
func RenderCommitBody(ctx context.Context, locale translation.Locale, msg, urlPrefix string, metas map[string]string) template.HTML {
771839
msgLine := strings.TrimRightFunc(msg, unicode.IsSpace)
772840
lineEnd := strings.IndexByte(msgLine, '\n')
773841
if lineEnd > 0 {
@@ -789,11 +857,12 @@ func RenderCommitBody(ctx context.Context, msg, urlPrefix string, metas map[stri
789857
log.Error("RenderCommitMessage: %v", err)
790858
return ""
791859
}
860+
_, renderedMessage = charset.EscapeControlHTML(renderedMessage, locale)
792861
return template.HTML(renderedMessage)
793862
}
794863

795864
// RenderIssueTitle renders issue/pull title with defined post processors
796-
func RenderIssueTitle(ctx context.Context, text, urlPrefix string, metas map[string]string) template.HTML {
865+
func RenderIssueTitle(ctx context.Context, locale translation.Locale, text, urlPrefix string, metas map[string]string) template.HTML {
797866
renderedText, err := markup.RenderIssueTitle(&markup.RenderContext{
798867
Ctx: ctx,
799868
URLPrefix: urlPrefix,
@@ -803,6 +872,7 @@ func RenderIssueTitle(ctx context.Context, text, urlPrefix string, metas map[str
803872
log.Error("RenderIssueTitle: %v", err)
804873
return template.HTML("")
805874
}
875+
_, renderedText = charset.EscapeControlHTML(renderedText, locale)
806876
return template.HTML(renderedText)
807877
}
808878

@@ -830,7 +900,7 @@ func ReactionToEmoji(reaction string) template.HTML {
830900
}
831901

832902
// RenderNote renders the contents of a git-notes file as a commit message.
833-
func RenderNote(ctx context.Context, msg, urlPrefix string, metas map[string]string) template.HTML {
903+
func RenderNote(ctx context.Context, locale translation.Locale, msg, urlPrefix string, metas map[string]string) template.HTML {
834904
cleanMsg := template.HTMLEscapeString(msg)
835905
fullMessage, err := markup.RenderCommitMessage(&markup.RenderContext{
836906
Ctx: ctx,
@@ -841,6 +911,8 @@ func RenderNote(ctx context.Context, msg, urlPrefix string, metas map[string]str
841911
log.Error("RenderNote: %v", err)
842912
return ""
843913
}
914+
_, fullMessage = charset.EscapeControlHTML(fullMessage, locale)
915+
844916
return template.HTML(fullMessage)
845917
}
846918

‎options/locale/locale_en-US.ini

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,9 @@ never = Never
106106

107107
rss_feed = RSS Feed
108108

109+
invisible_runes = `This field has invisible unicode characters`
110+
ambiguous_runes = `This field has ambiguous unicode characters`
111+
109112
[error]
110113
occurred = An error occurred
111114
report_message = If you are sure this is a Gitea bug, please search for issues on <a href="https://github.com/go-gitea/gitea/issues" target="_blank">GitHub</a> or open a new issue if necessary.

‎routers/web/feed/convert.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ func feedActionsToFeedItems(ctx *context.Context, actions models.ActionList) (it
202202
desc += fmt.Sprintf("<a href=\"%s\">%s</a>\n%s",
203203
html.EscapeString(fmt.Sprintf("%s/commit/%s", act.GetRepoLink(), commit.Sha1)),
204204
commit.Sha1,
205-
templates.RenderCommitMessage(ctx, commit.Message, repoLink, nil),
205+
templates.RenderCommitMessage(ctx, ctx.Locale, commit.Message, repoLink, nil),
206206
)
207207
}
208208

‎routers/web/repo/lfs.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -309,7 +309,7 @@ func LFSFileGet(ctx *context.Context) {
309309

310310
// Building code view blocks with line number on server side.
311311
escapedContent := &bytes.Buffer{}
312-
ctx.Data["EscapeStatus"], _ = charset.EscapeControlReader(rd, escapedContent, ctx.Locale)
312+
ctx.Data["EscapeStatus"], _ = charset.EscapeControlHTMLReader(rd, escapedContent, ctx.Locale)
313313

314314
var output bytes.Buffer
315315
lines := strings.Split(escapedContent.String(), "\n")

‎routers/web/repo/view.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -339,15 +339,15 @@ func renderReadmeFile(ctx *context.Context, readmeFile *namedBlob, readmeTreelin
339339
if err != nil {
340340
log.Error("Render failed for %s in %-v: %v Falling back to rendering source", readmeFile.name, ctx.Repo.Repository, err)
341341
buf := &bytes.Buffer{}
342-
ctx.Data["EscapeStatus"], _ = charset.EscapeControlReader(rd, buf, ctx.Locale)
342+
ctx.Data["EscapeStatus"], _ = charset.EscapeControlHTMLReader(rd, buf, ctx.Locale)
343343
ctx.Data["FileContent"] = strings.ReplaceAll(
344344
gotemplate.HTMLEscapeString(buf.String()), "\n", `<br>`,
345345
)
346346
}
347347
} else {
348348
ctx.Data["IsRenderedHTML"] = true
349349
buf := &bytes.Buffer{}
350-
ctx.Data["EscapeStatus"], err = charset.EscapeControlReader(rd, &charset.BreakWriter{Writer: buf}, ctx.Locale, charset.RuneNBSP)
350+
ctx.Data["EscapeStatus"], err = charset.EscapeControlHTMLReader(rd, &charset.BreakWriter{Writer: buf}, ctx.Locale, charset.RuneNBSP)
351351
if err != nil {
352352
log.Error("Read failed: %v", err)
353353
}
@@ -517,7 +517,7 @@ func renderFile(ctx *context.Context, entry *git.TreeEntry, treeLink, rawLink st
517517
buf := &bytes.Buffer{}
518518
ctx.Data["IsRenderedHTML"] = true
519519

520-
ctx.Data["EscapeStatus"], _ = charset.EscapeControlReader(rd, buf, ctx.Locale)
520+
ctx.Data["EscapeStatus"], _ = charset.EscapeControlHTMLReader(rd, buf, ctx.Locale)
521521

522522
ctx.Data["FileContent"] = strings.ReplaceAll(
523523
gotemplate.HTMLEscapeString(buf.String()), "\n", `<br>`,
@@ -644,7 +644,7 @@ func markupRender(ctx *context.Context, renderCtx *markup.RenderContext, input i
644644
go func() {
645645
sb := &strings.Builder{}
646646
// We allow NBSP here this is rendered
647-
escaped, _ = charset.EscapeControlReader(markupRd, sb, ctx.Locale, charset.RuneNBSP)
647+
escaped, _ = charset.EscapeControlHTMLReader(markupRd, sb, ctx.Locale, charset.RuneNBSP)
648648
output = sb.String()
649649
close(done)
650650
}()

‎routers/web/repo/wiki.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,7 @@ func renderViewPage(ctx *context.Context) (*git.Repository, *git.TreeEntry) {
247247
done := make(chan struct{})
248248
go func() {
249249
// We allow NBSP here this is rendered
250-
escaped, _ = charset.EscapeControlReader(markupRd, buf, ctx.Locale, charset.RuneNBSP)
250+
escaped, _ = charset.EscapeControlHTMLReader(markupRd, buf, ctx.Locale, charset.RuneNBSP)
251251
output = buf.String()
252252
buf.Reset()
253253
close(done)

‎templates/admin/emails/list.tmpl

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
</div>
3030
</form>
3131
</div>
32-
<div class="ui attached table segment">
32+
<div class="ui attached table segment unicode-escaped">
3333
<table class="ui very basic striped table unstackable">
3434
<thead>
3535
<tr>
@@ -50,8 +50,8 @@
5050
{{range .Emails}}
5151
<tr>
5252
<td><a href="{{AppSubUrl}}/{{.Name | PathEscape}}">{{.Name}}</a></td>
53-
<td><span class="text truncate">{{.FullName}}</span></td>
54-
<td><span class="text email">{{.Email}}</span></td>
53+
<td><span class="text truncate">{{escapeAmbiguous $.locale .FullName}}</span></td>
54+
<td><span class="text email">{{escapeAmbiguous $.locale .Email}}</span></td>
5555
<td>{{if .IsPrimary}}{{svg "octicon-check"}}{{else}}{{svg "octicon-x"}}{{end}}</td>
5656
<td>
5757
{{if .CanChange}}

‎templates/admin/user/list.tmpl

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@
6060
</div>
6161
</form>
6262
</div>
63-
<div class="ui attached table segment">
63+
<div class="ui attached table segment unicode-escaped">
6464
<table class="ui very basic striped table unstackable">
6565
<thead>
6666
<tr>
@@ -88,7 +88,7 @@
8888
<tr>
8989
<td>{{.ID}}</td>
9090
<td><a href="{{.HomeLink}}">{{.Name}}</a></td>
91-
<td><span class="text truncate email">{{.Email}}</span></td>
91+
<td><span class="text truncate email">{{escapeAmbiguous $.locale .Email}}</span></td>
9292
<td>{{if .IsActive}}{{svg "octicon-check"}}{{else}}{{svg "octicon-x"}}{{end}}</td>
9393
<td>{{if .IsAdmin}}{{svg "octicon-check"}}{{else}}{{svg "octicon-x"}}{{end}}</td>
9494
<td>{{if .IsRestricted}}{{svg "octicon-check"}}{{else}}{{svg "octicon-x"}}{{end}}</td>

‎templates/base/head_script.tmpl

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,15 @@ If you introduce mistakes in it, Gitea JavaScript code wouldn't run correctly.
1919
notificationSettings: {{NotificationSettings}}, {{/*a map provided by NewFuncMap in helper.go*/}}
2020
enableTimeTracking: {{EnableTimetracking}},
2121
{{if .RequireTribute}}
22+
{{- /* WARNING: fullname below is assumed to be safe for HTML in tribute.js do not add unescaped content as fullname */}}
2223
tributeValues: Array.from(new Map([
2324
{{ range .Participants }}
2425
['{{.Name}}', {key: '{{.Name}} {{.FullName}}', value: '{{.Name}}',
25-
name: '{{.Name}}', fullname: '{{.FullName}}', avatar: '{{.AvatarLink}}'}],
26+
name: '{{.Name}}', fullname: '{{escapeAmbiguous $.locale .FullName}}', avatar: '{{.AvatarLink}}'}],
2627
{{ end }}
2728
{{ range .Assignees }}
2829
['{{.Name}}', {key: '{{.Name}} {{.FullName}}', value: '{{.Name}}',
29-
name: '{{.Name}}', fullname: '{{.FullName}}', avatar: '{{.AvatarLink}}'}],
30+
name: '{{.Name}}', fullname: '{{escapeAmbiguous $.locale .FullName}}', avatar: '{{.AvatarLink}}'}],
3031
{{ end }}
3132
{{ range .MentionableTeams }}
3233
['{{$.MentionableTeamsOrg}}/{{.Name}}', {key: '{{$.MentionableTeamsOrg}}/{{.Name}}', value: '{{$.MentionableTeamsOrg}}/{{.Name}}',

‎templates/explore/organizations.tmpl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
{{avatar .}}
1111
<div class="content">
1212
<span class="header">
13-
<a href="{{.HomeLink}}">{{.Name}}</a> {{.FullName}}
13+
<a href="{{.HomeLink}}">{{.Name}}</a> {{escapeAmbiguous $.locale .FullName}}
1414
{{if .Visibility.IsPrivate}}
1515
<span class="ui basic label">{{$.locale.Tr "repo.desc.private"}}</span>
1616
{{end}}

0 commit comments

Comments
 (0)