WIP: Add Ambiguous Character Detection to Description, FullName and others

modules/charset/escape.go

@@ -22,18 +22,12 @@ const RuneNBSP = 0xa0
 // EscapeControlHTML escapes the unicode control sequences in a provided html document
 func EscapeControlHTML(text string, locale translation.Locale, allowed ...rune) (escaped *EscapeStatus, output string) {
 	sb := &strings.Builder{}
-	outputStream := &HTMLStreamerWriter{Writer: sb}
-	streamer := NewEscapeStreamer(locale, outputStream, allowed...).(*escapeStreamer)
-
-	if err := StreamHTML(strings.NewReader(text), streamer); err != nil {
-		streamer.escaped.HasError = true
-		log.Error("Error whilst escaping: %v", err)
-	}
-	return streamer.escaped, sb.String()
+	escaped, _ = EscapeControlHTMLReader(strings.NewReader(text), sb, locale, allowed...)
+	return escaped, sb.String()
 }
 
-// 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
-func EscapeControlReader(reader io.Reader, writer io.Writer, locale translation.Locale, allowed ...rune) (escaped *EscapeStatus, err error) {
+// 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
+func EscapeControlHTMLReader(reader io.Reader, writer io.Writer, locale translation.Locale, allowed ...rune) (escaped *EscapeStatus, err error) {
 	outputStream := &HTMLStreamerWriter{Writer: writer}
 	streamer := NewEscapeStreamer(locale, outputStream, allowed...).(*escapeStreamer)
 
@@ -56,3 +50,15 @@ func EscapeControlString(text string, locale translation.Locale, allowed ...rune
 	}
 	return streamer.escaped, sb.String()
 }
+
+// EscapeControlStringWriter escapes the unicode control sequences in a string and provided writer in a locale and returns the findings as an EscapeStatus
+func EscapeControlStringWriter(text string, writer io.Writer, locale translation.Locale, allowed ...rune) (escaped *EscapeStatus, err error) {
+	outputStream := &HTMLStreamerWriter{Writer: writer}
+	streamer := NewEscapeStreamer(locale, outputStream, allowed...).(*escapeStreamer)
+
+	if err = streamer.Text(text); err != nil {
+		streamer.escaped.HasError = true
+		log.Error("Error whilst escaping: %v", err)
+	}
+	return streamer.escaped, err
+}

modules/charset/escape_test.go

@@ -173,7 +173,7 @@ func TestEscapeControlReader(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			input := strings.NewReader(tt.text)
 			output := &strings.Builder{}
-			status, err := EscapeControlReader(input, output, translation.NewLocale("en_US"))
+			status, err := EscapeControlHTMLReader(input, output, translation.NewLocale("en_US"))
 			result := output.String()
 			if err != nil {
 				t.Errorf("EscapeControlReader(): err = %v", err)

modules/templates/helper.go

@@ -31,6 +31,7 @@ import (
 	repo_model "code.gitea.io/gitea/models/repo"
 	user_model "code.gitea.io/gitea/models/user"
 	"code.gitea.io/gitea/modules/base"
+	"code.gitea.io/gitea/modules/charset"
 	"code.gitea.io/gitea/modules/emoji"
 	"code.gitea.io/gitea/modules/git"
 	giturl "code.gitea.io/gitea/modules/git/url"
@@ -42,6 +43,7 @@ import (
 	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/svg"
 	"code.gitea.io/gitea/modules/timeutil"
+	"code.gitea.io/gitea/modules/translation"
 	"code.gitea.io/gitea/modules/util"
 	"code.gitea.io/gitea/services/gitdiff"
 
@@ -343,12 +345,15 @@ func NewFuncMap() []template.FuncMap {
 			}
 			return false
 		},
-		"svg":            SVG,
-		"avatar":         Avatar,
-		"avatarHTML":     AvatarHTML,
-		"avatarByAction": AvatarByAction,
-		"avatarByEmail":  AvatarByEmail,
-		"repoAvatar":     RepoAvatar,
+		"svg":                 SVG,
+		"avatar":              Avatar,
+		"avatarHTML":          AvatarHTML,
+		"avatarByAction":      AvatarByAction,
+		"avatarByEmail":       AvatarByEmail,
+		"escapeAmbiguous":     EscapeAmbiguous,
+		"escapeAmbiguousHTML": EscapeAmbiguousHTML,
+		"escapeAmbiguousLink": EscapeAmbiguousLink,
+		"repoAvatar":          RepoAvatar,
 		"SortArrow": func(normSort, revSort, urlSort string, isDefault bool) template.HTML {
 			// if needed
 			if len(normSort) == 0 || len(urlSort) == 0 {
@@ -680,6 +685,67 @@ func AvatarByEmail(email, name string, others ...interface{}) template.HTML {
 	return template.HTML("")
 }
 
+// EscapeAmbiguous
+func EscapeAmbiguous(locale translation.Locale, text string) template.HTML {
+	sb := &strings.Builder{}
+	status, _ := charset.EscapeControlStringWriter(text, sb, locale)
+	escapeStatusSwitch(locale, sb, status)
+
+	return template.HTML(sb.String())
+}
+
+// EscapeAmbiguousHTML
+func EscapeAmbiguousHTML(locale translation.Locale, html string) template.HTML {
+	sb := &strings.Builder{}
+	status, _ := charset.EscapeControlHTMLReader(strings.NewReader(html), sb, locale)
+	escapeStatusSwitch(locale, sb, status)
+	return template.HTML(sb.String())
+}
+
+// EscapeAmbiguousLink takes a locale, text body - which is assumed to be a string not html, href and other attributes
+func EscapeAmbiguousLink(locale translation.Locale, text, href string, attrs ...string) template.HTML {
+	sb := &strings.Builder{}
+	_, _ = sb.WriteString(`<a href="`)
+	template.HTMLEscape(sb, []byte(href))
+	_, _ = sb.WriteString(`"`)
+	attrValue := false
+	for _, attr := range attrs {
+		if attrValue {
+			_, _ = sb.WriteString(`="`)
+		} else {
+			_, _ = sb.WriteString(` `)
+		}
+		template.HTMLEscape(sb, []byte(attr))
+		if attrValue {
+			_, _ = sb.WriteString(`"`)
+		}
+		attrValue = !attrValue
+	}
+	_, _ = sb.WriteString(`>`)
+	status, _ := charset.EscapeControlStringWriter(text, sb, locale)
+	_, _ = sb.WriteString(`</a>`)
+
+	escapeStatusSwitch(locale, sb, status)
+	return template.HTML(sb.String())
+}
+
+func escapeStatusSwitch(locale translation.Locale, sb *strings.Builder, status *charset.EscapeStatus) {
+	if status.Escaped {
+		_, _ = sb.WriteString(`<a href="" class="toggle-escape-button" title="`)
+		if status.HasInvisible {
+			_, _ = sb.WriteString(locale.Tr("invisible_runes"))
+		}
+		if status.HasInvisible && status.HasAmbiguous {
+			_, _ = sb.WriteString(` `)
+		}
+		if status.HasAmbiguous {
+			_, _ = sb.WriteString(locale.Tr("ambiguous_runes"))
+		}
+
+		_, _ = sb.WriteString(`"></a>`)
+	}
+}
+
 // Safe render raw as HTML
 func Safe(raw string) template.HTML {
 	return template.HTML(raw)
@@ -711,13 +777,13 @@ func DotEscape(raw string) string {
 }
 
 // RenderCommitMessage renders commit message with XSS-safe and special links.
-func RenderCommitMessage(ctx context.Context, msg, urlPrefix string, metas map[string]string) template.HTML {
-	return RenderCommitMessageLink(ctx, msg, urlPrefix, "", metas)
+func RenderCommitMessage(ctx context.Context, locale translation.Locale, msg, urlPrefix string, metas map[string]string) template.HTML {
+	return RenderCommitMessageLink(ctx, locale, msg, urlPrefix, "", metas)
 }
 
 // RenderCommitMessageLink renders commit message as a XXS-safe link to the provided
 // default url, handling for special links.
-func RenderCommitMessageLink(ctx context.Context, msg, urlPrefix, urlDefault string, metas map[string]string) template.HTML {
+func RenderCommitMessageLink(ctx context.Context, locale translation.Locale, msg, urlPrefix, urlDefault string, metas map[string]string) template.HTML {
 	cleanMsg := template.HTMLEscapeString(msg)
 	// we can safely assume that it will not return any error, since there
 	// shouldn't be any special HTML.
@@ -731,16 +797,17 @@ func RenderCommitMessageLink(ctx context.Context, msg, urlPrefix, urlDefault str
 		log.Error("RenderCommitMessage: %v", err)
 		return ""
 	}
-	msgLines := strings.Split(strings.TrimSpace(fullMessage), "\n")
+	msgLines := strings.SplitN(strings.TrimSpace(fullMessage), "\n", 2)
 	if len(msgLines) == 0 {
 		return template.HTML("")
 	}
-	return template.HTML(msgLines[0])
+	_, renderedMessage := charset.EscapeControlHTML(msgLines[0], locale)
+	return template.HTML(renderedMessage)
 }
 
 // RenderCommitMessageLinkSubject renders commit message as a XXS-safe link to
 // the provided default url, handling for special links without email to links.
-func RenderCommitMessageLinkSubject(ctx context.Context, msg, urlPrefix, urlDefault string, metas map[string]string) template.HTML {
+func RenderCommitMessageLinkSubject(ctx context.Context, locale translation.Locale, msg, urlPrefix, urlDefault string, metas map[string]string) template.HTML {
 	msgLine := strings.TrimLeftFunc(msg, unicode.IsSpace)
 	lineEnd := strings.IndexByte(msgLine, '\n')
 	if lineEnd > 0 {
@@ -763,11 +830,12 @@ func RenderCommitMessageLinkSubject(ctx context.Context, msg, urlPrefix, urlDefa
 		log.Error("RenderCommitMessageSubject: %v", err)
 		return template.HTML("")
 	}
+	_, renderedMessage = charset.EscapeControlHTML(renderedMessage, locale)
 	return template.HTML(renderedMessage)
 }
 
 // RenderCommitBody extracts the body of a commit message without its title.
-func RenderCommitBody(ctx context.Context, msg, urlPrefix string, metas map[string]string) template.HTML {
+func RenderCommitBody(ctx context.Context, locale translation.Locale, msg, urlPrefix string, metas map[string]string) template.HTML {
 	msgLine := strings.TrimRightFunc(msg, unicode.IsSpace)
 	lineEnd := strings.IndexByte(msgLine, '\n')
 	if lineEnd > 0 {
@@ -789,11 +857,12 @@ func RenderCommitBody(ctx context.Context, msg, urlPrefix string, metas map[stri
 		log.Error("RenderCommitMessage: %v", err)
 		return ""
 	}
+	_, renderedMessage = charset.EscapeControlHTML(renderedMessage, locale)
 	return template.HTML(renderedMessage)
 }
 
 // RenderIssueTitle renders issue/pull title with defined post processors
-func RenderIssueTitle(ctx context.Context, text, urlPrefix string, metas map[string]string) template.HTML {
+func RenderIssueTitle(ctx context.Context, locale translation.Locale, text, urlPrefix string, metas map[string]string) template.HTML {
 	renderedText, err := markup.RenderIssueTitle(&markup.RenderContext{
 		Ctx:       ctx,
 		URLPrefix: urlPrefix,
@@ -803,6 +872,7 @@ func RenderIssueTitle(ctx context.Context, text, urlPrefix string, metas map[str
 		log.Error("RenderIssueTitle: %v", err)
 		return template.HTML("")
 	}
+	_, renderedText = charset.EscapeControlHTML(renderedText, locale)
 	return template.HTML(renderedText)
 }
 
@@ -830,7 +900,7 @@ func ReactionToEmoji(reaction string) template.HTML {
 }
 
 // RenderNote renders the contents of a git-notes file as a commit message.
-func RenderNote(ctx context.Context, msg, urlPrefix string, metas map[string]string) template.HTML {
+func RenderNote(ctx context.Context, locale translation.Locale, msg, urlPrefix string, metas map[string]string) template.HTML {
 	cleanMsg := template.HTMLEscapeString(msg)
 	fullMessage, err := markup.RenderCommitMessage(&markup.RenderContext{
 		Ctx:       ctx,
@@ -841,6 +911,8 @@ func RenderNote(ctx context.Context, msg, urlPrefix string, metas map[string]str
 		log.Error("RenderNote: %v", err)
 		return ""
 	}
+	_, fullMessage = charset.EscapeControlHTML(fullMessage, locale)
+
 	return template.HTML(fullMessage)
 }
 

options/locale/locale_en-US.ini

@@ -106,6 +106,9 @@ never = Never
 
 rss_feed = RSS Feed
 
+invisible_runes = `This field has invisible unicode characters`
+ambiguous_runes = `This field has ambiguous unicode characters`
+
 [error]
 occurred = An error occurred
 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

@@ -202,7 +202,7 @@ func feedActionsToFeedItems(ctx *context.Context, actions models.ActionList) (it
 					desc += fmt.Sprintf("<a href=\"%s\">%s</a>\n%s",
 						html.EscapeString(fmt.Sprintf("%s/commit/%s", act.GetRepoLink(), commit.Sha1)),
 						commit.Sha1,
-						templates.RenderCommitMessage(ctx, commit.Message, repoLink, nil),
+						templates.RenderCommitMessage(ctx, ctx.Locale, commit.Message, repoLink, nil),
 					)
 				}
 

routers/web/repo/lfs.go

@@ -309,7 +309,7 @@ func LFSFileGet(ctx *context.Context) {
 
 		// Building code view blocks with line number on server side.
 		escapedContent := &bytes.Buffer{}
-		ctx.Data["EscapeStatus"], _ = charset.EscapeControlReader(rd, escapedContent, ctx.Locale)
+		ctx.Data["EscapeStatus"], _ = charset.EscapeControlHTMLReader(rd, escapedContent, ctx.Locale)
 
 		var output bytes.Buffer
 		lines := strings.Split(escapedContent.String(), "\n")

routers/web/repo/view.go

@@ -339,15 +339,15 @@ func renderReadmeFile(ctx *context.Context, readmeFile *namedBlob, readmeTreelin
 		if err != nil {
 			log.Error("Render failed for %s in %-v: %v Falling back to rendering source", readmeFile.name, ctx.Repo.Repository, err)
 			buf := &bytes.Buffer{}
-			ctx.Data["EscapeStatus"], _ = charset.EscapeControlReader(rd, buf, ctx.Locale)
+			ctx.Data["EscapeStatus"], _ = charset.EscapeControlHTMLReader(rd, buf, ctx.Locale)
 			ctx.Data["FileContent"] = strings.ReplaceAll(
 				gotemplate.HTMLEscapeString(buf.String()), "\n", `<br>`,
 			)
 		}
 	} else {
 		ctx.Data["IsRenderedHTML"] = true
 		buf := &bytes.Buffer{}
-		ctx.Data["EscapeStatus"], err = charset.EscapeControlReader(rd, &charset.BreakWriter{Writer: buf}, ctx.Locale, charset.RuneNBSP)
+		ctx.Data["EscapeStatus"], err = charset.EscapeControlHTMLReader(rd, &charset.BreakWriter{Writer: buf}, ctx.Locale, charset.RuneNBSP)
 		if err != nil {
 			log.Error("Read failed: %v", err)
 		}
@@ -517,7 +517,7 @@ func renderFile(ctx *context.Context, entry *git.TreeEntry, treeLink, rawLink st
 			buf := &bytes.Buffer{}
 			ctx.Data["IsRenderedHTML"] = true
 
-			ctx.Data["EscapeStatus"], _ = charset.EscapeControlReader(rd, buf, ctx.Locale)
+			ctx.Data["EscapeStatus"], _ = charset.EscapeControlHTMLReader(rd, buf, ctx.Locale)
 
 			ctx.Data["FileContent"] = strings.ReplaceAll(
 				gotemplate.HTMLEscapeString(buf.String()), "\n", `<br>`,
@@ -644,7 +644,7 @@ func markupRender(ctx *context.Context, renderCtx *markup.RenderContext, input i
 	go func() {
 		sb := &strings.Builder{}
 		// We allow NBSP here this is rendered
-		escaped, _ = charset.EscapeControlReader(markupRd, sb, ctx.Locale, charset.RuneNBSP)
+		escaped, _ = charset.EscapeControlHTMLReader(markupRd, sb, ctx.Locale, charset.RuneNBSP)
 		output = sb.String()
 		close(done)
 	}()

routers/web/repo/wiki.go

@@ -247,7 +247,7 @@ func renderViewPage(ctx *context.Context) (*git.Repository, *git.TreeEntry) {
 		done := make(chan struct{})
 		go func() {
 			// We allow NBSP here this is rendered
-			escaped, _ = charset.EscapeControlReader(markupRd, buf, ctx.Locale, charset.RuneNBSP)
+			escaped, _ = charset.EscapeControlHTMLReader(markupRd, buf, ctx.Locale, charset.RuneNBSP)
 			output = buf.String()
 			buf.Reset()
 			close(done)

templates/admin/emails/list.tmpl

@@ -29,7 +29,7 @@
 				</div>
 			</form>
 		</div>
-		<div class="ui attached table segment">
+		<div class="ui attached table segment unicode-escaped">
 			<table class="ui very basic striped table unstackable">
 				<thead>
 					<tr>
@@ -50,8 +50,8 @@
 					{{range .Emails}}
 						<tr>
 							<td><a href="{{AppSubUrl}}/{{.Name | PathEscape}}">{{.Name}}</a></td>
-							<td><span class="text truncate">{{.FullName}}</span></td>
-							<td><span class="text email">{{.Email}}</span></td>
+							<td><span class="text truncate">{{escapeAmbiguous $.locale .FullName}}</span></td>
+							<td><span class="text email">{{escapeAmbiguous $.locale .Email}}</span></td>
 							<td>{{if .IsPrimary}}{{svg "octicon-check"}}{{else}}{{svg "octicon-x"}}{{end}}</td>
 							<td>
 								{{if .CanChange}}

templates/admin/user/list.tmpl

@@ -60,7 +60,7 @@
 				</div>
 			</form>
 		</div>
-		<div class="ui attached table segment">
+		<div class="ui attached table segment unicode-escaped">
 			<table class="ui very basic striped table unstackable">
 				<thead>
 					<tr>
@@ -88,7 +88,7 @@
 						<tr>
 							<td>{{.ID}}</td>
 							<td><a href="{{.HomeLink}}">{{.Name}}</a></td>
-							<td><span class="text truncate email">{{.Email}}</span></td>
+							<td><span class="text truncate email">{{escapeAmbiguous $.locale .Email}}</span></td>
 							<td>{{if .IsActive}}{{svg "octicon-check"}}{{else}}{{svg "octicon-x"}}{{end}}</td>
 							<td>{{if .IsAdmin}}{{svg "octicon-check"}}{{else}}{{svg "octicon-x"}}{{end}}</td>
 							<td>{{if .IsRestricted}}{{svg "octicon-check"}}{{else}}{{svg "octicon-x"}}{{end}}</td>

templates/base/head_script.tmpl

@@ -19,14 +19,15 @@ If you introduce mistakes in it, Gitea JavaScript code wouldn't run correctly.
 		notificationSettings: {{NotificationSettings}}, {{/*a map provided by NewFuncMap in helper.go*/}}
 		enableTimeTracking: {{EnableTimetracking}},
 		{{if .RequireTribute}}
+		{{- /* WARNING: fullname below is assumed to be safe for HTML in tribute.js do not add unescaped content as fullname */}}
 		tributeValues: Array.from(new Map([
 			{{ range .Participants }}
 			['{{.Name}}', {key: '{{.Name}} {{.FullName}}', value: '{{.Name}}',
-			name: '{{.Name}}', fullname: '{{.FullName}}', avatar: '{{.AvatarLink}}'}],
+			name: '{{.Name}}', fullname: '{{escapeAmbiguous $.locale .FullName}}', avatar: '{{.AvatarLink}}'}],
 			{{ end }}
 			{{ range .Assignees }}
 			['{{.Name}}', {key: '{{.Name}} {{.FullName}}', value: '{{.Name}}',
-			name: '{{.Name}}', fullname: '{{.FullName}}', avatar: '{{.AvatarLink}}'}],
+			name: '{{.Name}}', fullname: '{{escapeAmbiguous $.locale .FullName}}', avatar: '{{.AvatarLink}}'}],
 			{{ end }}
 			{{ range .MentionableTeams }}
 				['{{$.MentionableTeamsOrg}}/{{.Name}}', {key: '{{$.MentionableTeamsOrg}}/{{.Name}}', value: '{{$.MentionableTeamsOrg}}/{{.Name}}',

templates/explore/organizations.tmpl

@@ -10,7 +10,7 @@
 					{{avatar .}}
 					<div class="content">
 						<span class="header">
-							<a href="{{.HomeLink}}">{{.Name}}</a> {{.FullName}}
+							<a href="{{.HomeLink}}">{{.Name}}</a> {{escapeAmbiguous $.locale .FullName}}
 							{{if .Visibility.IsPrivate}}
 								<span class="ui basic label">{{$.locale.Tr "repo.desc.private"}}</span>
 							{{end}}