Add warning for BIDI characters in page renders and in diffs

Fix go-gitea#17514

Signed-off-by: Andrew Thornton <art27@cantab.net>

modules/charset/charset.go

@@ -23,6 +23,63 @@ import (
 // UTF8BOM is the utf-8 byte-order marker
 var UTF8BOM = []byte{'\xef', '\xbb', '\xbf'}
 
+// BIDIRunes are runes that are explicitly mentioned in CVE-2021-42574
+var BIDIRunes = "\u202A\u202B\u202C\u202D\u202E\u2066\u2067\u2068\u2069"
+
+// ContainsBIDIRuneString checks some text for any bidi rune
+func ContainsBIDIRuneString(text string) bool {
+	return strings.ContainsAny(text, BIDIRunes)
+}
+
+// ContainsBIDIRuneBytes checks some text for any bidi rune
+func ContainsBIDIRuneBytes(text []byte) bool {
+	return bytes.ContainsAny(text, BIDIRunes)
+}
+
+// ContainsBIDIRuneReader checks some text for any bidi rune
+func ContainsBIDIRuneReader(text io.Reader) (bool, error) {
+	buf := make([]byte, 4096)
+	readStart := 0
+	var err error
+	var n int
+	for err == nil {
+		n, err = text.Read(buf[readStart:])
+		bs := buf[:n]
+		i := 0
+	inner:
+		for i < n {
+			r, size := utf8.DecodeRune(bs[i:])
+			if r == utf8.RuneError {
+				// need to decide what to do here... runes can be at most 4 bytes - so... i123n
+				if n-i > 3 {
+					// this is a real broken rune
+					return true, fmt.Errorf("text contains bad rune: %x", bs[i])
+				}
+
+				break inner
+			}
+			if strings.ContainsRune(BIDIRunes, r) {
+				return true, nil
+			}
+			i += size
+		}
+		if n > 0 {
+			readStart = 0
+		}
+		if i < n {
+			copy(buf, bs[i:n])
+			readStart = n - i
+		}
+	}
+	if readStart > 0 {
+		return true, fmt.Errorf("text contains bad rune: %x", buf[0])
+	}
+	if err == io.EOF {
+		return false, nil
+	}
+	return true, err
+}
+
 // ToUTF8WithFallbackReader detects the encoding of content and coverts to UTF-8 reader if possible
 func ToUTF8WithFallbackReader(rd io.Reader) io.Reader {
 	var buf = make([]byte, 2048)

modules/charset/charset_test.go

@@ -5,11 +5,11 @@
 package charset
 
 import (
+	"bytes"
 	"strings"
 	"testing"
 
 	"code.gitea.io/gitea/modules/setting"
-
 	"github.com/stretchr/testify/assert"
 )
 
@@ -272,3 +272,94 @@ func stringMustEndWith(t *testing.T, expected string, value string) {
 func bytesMustStartWith(t *testing.T, expected []byte, value []byte) {
 	assert.Equal(t, expected, value[:len(expected)])
 }
+
+var bidiTestCases = []struct {
+	name string
+	text string
+	want bool
+}{
+	{
+		name: "Accented characters",
+		text: string([]byte{0xc3, 0xa1, 0xc3, 0xa9, 0xc3, 0xad, 0xc3, 0xb3, 0xc3, 0xba}),
+		want: false,
+	},
+	{
+		name: "Program",
+		text: "string([]byte{0xc3, 0xa1, 0xc3, 0xa9, 0xc3, 0xad, 0xc3, 0xb3, 0xc3, 0xba})",
+		want: false,
+	},
+	{
+		name: "CVE testcase",
+		text: "if access_level != \"user\u202E \u2066// Check if admin\u2069 \u2066\" {",
+		want: true,
+	},
+}
+
+func TestContainsBIDIRuneString(t *testing.T) {
+	for _, tt := range bidiTestCases {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := ContainsBIDIRuneString(tt.text); got != tt.want {
+				t.Errorf("ContainsBIDIRuneString() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
+
+func TestContainsBIDIRuneBytes(t *testing.T) {
+	for _, tt := range bidiTestCases {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := ContainsBIDIRuneBytes([]byte(tt.text)); got != tt.want {
+				t.Errorf("ContainsBIDIRuneBytes() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
+
+func TestContainsBIDIRuneReader(t *testing.T) {
+	for _, tt := range bidiTestCases {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := ContainsBIDIRuneReader(strings.NewReader(tt.text))
+			if err != nil {
+				t.Errorf("ContainsBIDIRuneReader() error = %v", err)
+				return
+			}
+			if got != tt.want {
+				t.Errorf("ContainsBIDIRuneReader() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+
+	// now we need some specific tests with broken runes
+	for _, tt := range bidiTestCases {
+		t.Run(tt.name+"-terminal-xc2", func(t *testing.T) {
+			bs := []byte(tt.text)
+			bs = append(bs, 0xc2)
+			got, err := ContainsBIDIRuneReader(bytes.NewReader(bs))
+			if !tt.want {
+				if err == nil {
+					t.Errorf("ContainsBIDIRuneReader() should have errored")
+					return
+				}
+			} else if !got {
+				t.Errorf("ContainsBIDIRuneReader() should have returned true")
+				return
+			}
+		})
+	}
+
+	for _, tt := range bidiTestCases {
+		t.Run(tt.name+"-prefix-xff", func(t *testing.T) {
+			bs := append([]byte{0xff}, []byte(tt.text)...)
+			got, err := ContainsBIDIRuneReader(bytes.NewReader(bs))
+			if err == nil {
+				t.Errorf("ContainsBIDIRuneReader() should have errored")
+				return
+			}
+			if !got {
+				t.Errorf("ContainsBIDIRuneReader() should have returned true")
+				return
+			}
+		})
+	}
+
+}

options/locale/locale_en-US.ini

@@ -977,6 +977,8 @@ file_view_rendered = View Rendered
 file_view_raw = View Raw
 file_permalink = Permalink
 file_too_large = The file is too large to be shown.
+bidi_header = `This file contains hidden Unicode characters!`
+bidi_description = `This file contains hidden Unicode characters that may be processed differently from what appears below.<br>If your use case is intentional and legitimate, you can safely ignore this warning.<br>Consult documentation of your favorite text editor for how to open this file using `DOS (CP 437)` encoding instead of Unicode, to reveal hidden characters.`
 file_copy_permalink = Copy Permalink
 video_not_supported_in_browser = Your browser does not support the HTML5 'video' tag.
 audio_not_supported_in_browser = Your browser does not support the HTML5 'audio' tag.
@@ -2057,6 +2059,7 @@ diff.protected = Protected
 diff.image.side_by_side = Side by Side
 diff.image.swipe = Swipe
 diff.image.overlay = Overlay
+diff.has_bidi = This line has hidden Unicode characters
 
 releases.desc = Track project versions and downloads.
 release.releases = Releases

routers/web/repo/lfs.go

@@ -316,6 +316,7 @@ func LFSFileGet(ctx *context.Context) {
 			output.WriteString(fmt.Sprintf(`<li class="L%d" rel="L%d">%s</li>`, index+1, index+1, line))
 		}
 		ctx.Data["FileContent"] = gotemplate.HTML(output.String())
+		ctx.Data["HasBIDI"] = charset.ContainsBIDIRuneString(output.String())
 
 		output.Reset()
 		for i := 0; i < len(lines); i++ {

routers/web/repo/view.go

@@ -332,18 +332,21 @@ func renderDirectory(ctx *context.Context, treeLink string) {
 					if err != nil {
 						log.Error("Render failed: %v then fallback", err)
 						bs, _ := io.ReadAll(rd)
+						ctx.Data["HasBIDI"] = charset.ContainsBIDIRuneBytes(bs)
 						ctx.Data["FileContent"] = strings.ReplaceAll(
 							gotemplate.HTMLEscapeString(string(bs)), "\n", `<br>`,
 						)
 					} else {
 						ctx.Data["FileContent"] = result.String()
+						ctx.Data["HasBIDI"] = charset.ContainsBIDIRuneString(result.String())
 					}
 				} else {
 					ctx.Data["IsRenderedHTML"] = true
 					buf, err = io.ReadAll(rd)
 					if err != nil {
 						log.Error("ReadAll failed: %v", err)
 					}
+					ctx.Data["HasBIDI"] = charset.ContainsBIDIRuneBytes(buf)
 					ctx.Data["FileContent"] = strings.ReplaceAll(
 						gotemplate.HTMLEscapeString(string(buf)), "\n", `<br>`,
 					)
@@ -490,9 +493,11 @@ func renderFile(ctx *context.Context, entry *git.TreeEntry, treeLink, rawLink st
 				return
 			}
 			ctx.Data["FileContent"] = result.String()
+			ctx.Data["HasBIDI"] = charset.ContainsBIDIRuneString(result.String())
 		} else if readmeExist {
 			buf, _ := io.ReadAll(rd)
 			ctx.Data["IsRenderedHTML"] = true
+			ctx.Data["HasBIDI"] = charset.ContainsBIDIRuneBytes(buf)
 			ctx.Data["FileContent"] = strings.ReplaceAll(
 				gotemplate.HTMLEscapeString(string(buf)), "\n", `<br>`,
 			)
@@ -501,6 +506,7 @@ func renderFile(ctx *context.Context, entry *git.TreeEntry, treeLink, rawLink st
 			lineNums := linesBytesCount(buf)
 			ctx.Data["NumLines"] = strconv.Itoa(lineNums)
 			ctx.Data["NumLinesSet"] = true
+			ctx.Data["HasBIDI"] = charset.ContainsBIDIRuneBytes(buf)
 			ctx.Data["FileContent"] = highlight.File(lineNums, blob.Name(), buf)
 		}
 		if !isLFSFile {
@@ -550,6 +556,7 @@ func renderFile(ctx *context.Context, entry *git.TreeEntry, treeLink, rawLink st
 				return
 			}
 			ctx.Data["FileContent"] = result.String()
+			ctx.Data["HasBIDI"] = charset.ContainsBIDIRuneString(result.String())
 		}
 	}
 

routers/web/repo/wiki.go

@@ -16,6 +16,7 @@ import (
 
 	"code.gitea.io/gitea/models"
 	"code.gitea.io/gitea/modules/base"
+	"code.gitea.io/gitea/modules/charset"
 	"code.gitea.io/gitea/modules/context"
 	"code.gitea.io/gitea/modules/git"
 	"code.gitea.io/gitea/modules/log"
@@ -232,6 +233,7 @@ func renderViewPage(ctx *context.Context) (*git.Repository, *git.TreeEntry) {
 		return nil, nil
 	}
 	ctx.Data["content"] = buf.String()
+	ctx.Data["HasBIDI"] = charset.ContainsBIDIRuneString(buf.String())
 
 	buf.Reset()
 	if err := markdown.Render(rctx, bytes.NewReader(sidebarContent), &buf); err != nil {
@@ -243,6 +245,7 @@ func renderViewPage(ctx *context.Context) (*git.Repository, *git.TreeEntry) {
 	}
 	ctx.Data["sidebarPresent"] = sidebarContent != nil
 	ctx.Data["sidebarContent"] = buf.String()
+	ctx.Data["sidebarHasBIDI"] = charset.ContainsBIDIRuneString(buf.String())
 
 	buf.Reset()
 	if err := markdown.Render(rctx, bytes.NewReader(footerContent), &buf); err != nil {
@@ -254,6 +257,7 @@ func renderViewPage(ctx *context.Context) (*git.Repository, *git.TreeEntry) {
 	}
 	ctx.Data["footerPresent"] = footerContent != nil
 	ctx.Data["footerContent"] = buf.String()
+	ctx.Data["footerHasBIDI"] = charset.ContainsBIDIRuneString(buf.String())
 
 	// get commit count - wiki revisions
 	commitsCount, _ := wikiRepo.FileCommitsCount("master", pageFilename)

services/gitdiff/gitdiff.go

@@ -117,6 +117,27 @@ func (d *DiffLine) GetCommentSide() string {
 	return d.Comments[0].DiffSide()
 }
 
+// HasBIDI returns true if there is a BIDI rune in this line
+func (d *DiffLine) HasBIDI() bool {
+	return charset.ContainsBIDIRuneString(d.Content)
+}
+
+// LeftHasBIDI returns true if there is a BIDI rune in this line
+func (d *DiffLine) LeftHasBIDI() bool {
+	if d.LeftIdx > 0 {
+		return charset.ContainsBIDIRuneString(d.Content)
+	}
+	return false
+}
+
+// RightHasBIDI returns true if there is a BIDI rune in this line
+func (d *DiffLine) RightHasBIDI() bool {
+	if d.RightIdx > 0 {
+		return charset.ContainsBIDIRuneString(d.Content)
+	}
+	return false
+}
+
 // GetLineTypeMarker returns the line type marker
 func (d *DiffLine) GetLineTypeMarker() string {
 	if strings.IndexByte(" +-", d.Content[0]) > -1 {

templates/repo/diff/section_split.tmpl

@@ -22,22 +22,22 @@
 							</a>
 						{{end}}
 					</td>
-					<td colspan="5" class="lines-code lines-code-old "><code class="code-inner">{{$section.GetComputedInlineDiffFor $line}}</span></td>
+					<td colspan="5" class="lines-code lines-code-old ">{{if $line.LeftHasBIDI}}<a class="ui button code-has-bidi" title="{{$.root.i18n.Tr "repo.diff.has_bidi"}}">&#xfe0f;&#x26a0;</a>{{end}}<code class="code-inner">{{$section.GetComputedInlineDiffFor $line}}</span></td>
 				{{else if and (eq .GetType 3) $hasmatch}}{{/* DEL */}}
 					{{$match := index $section.Lines $line.Match}}
 					<td class="lines-num lines-num-old del-code" data-line-num="{{$line.LeftIdx}}"><span rel="diff-{{Sha1 $file.Name}}L{{$line.LeftIdx}}"></span></td>
 					<td class="lines-type-marker lines-type-marker-old del-code"><span class="mono" data-type-marker="{{$line.GetLineTypeMarker}}"></span></td>
-					<td class="lines-code lines-code-old halfwidth del-code">{{if and $.root.SignedUserID $.root.PageIsPullFiles}}<a class="ui primary button add-code-comment add-code-comment-left{{if (not $line.CanComment)}} invisible{{end}}" data-path="{{$file.Name}}" data-side="left" data-idx="{{$line.LeftIdx}}" data-new-comment-url="{{$.root.Issue.HTMLURL}}/files/reviews/new_comment">{{svg "octicon-plus"}}</a>{{end}}<code class="code-inner">{{if $line.LeftIdx}}{{$section.GetComputedInlineDiffFor $line}}{{end}}</code></td>
+					<td class="lines-code lines-code-old halfwidth del-code">{{if and $.root.SignedUserID $.root.PageIsPullFiles}}<a class="ui primary button add-code-comment add-code-comment-left{{if (not $line.CanComment)}} invisible{{end}}" data-path="{{$file.Name}}" data-side="left" data-idx="{{$line.LeftIdx}}" data-new-comment-url="{{$.root.Issue.HTMLURL}}/files/reviews/new_comment">{{svg "octicon-plus"}}</a>{{end}}{{if $line.LeftHasBIDI}}<a class="ui button code-has-bidi" title="{{$.root.i18n.Tr "repo.diff.has_bidi"}}">&#xfe0f;&#x26a0;</a>{{end}}<code class="code-inner">{{if $line.LeftIdx}}{{$section.GetComputedInlineDiffFor $line}}{{end}}</code></td>
 					<td class="lines-num lines-num-new add-code" data-line-num="{{if $match.RightIdx}}{{$match.RightIdx}}{{end}}"><span rel="{{if $match.RightIdx}}diff-{{Sha1 $file.Name}}R{{$match.RightIdx}}{{end}}"></span></td>
 					<td class="lines-type-marker lines-type-marker-new add-code">{{if $match.RightIdx}}<span class="mono" data-type-marker="{{$match.GetLineTypeMarker}}"></span>{{end}}</td>
-					<td class="lines-code lines-code-new halfwidth add-code">{{if and $.root.SignedUserID $.root.PageIsPullFiles}}<a class="ui primary button add-code-comment add-code-comment-right{{if (not $match.CanComment)}} invisible{{end}}" data-path="{{$file.Name}}" data-side="right" data-idx="{{$match.RightIdx}}" data-new-comment-url="{{$.root.Issue.HTMLURL}}/files/reviews/new_comment">{{svg "octicon-plus"}}</a>{{end}}<code class="code-inner">{{if $match.RightIdx}}{{$section.GetComputedInlineDiffFor $match}}{{end}}</code></td>
+					<td class="lines-code lines-code-new halfwidth add-code">{{if and $.root.SignedUserID $.root.PageIsPullFiles}}<a class="ui primary button add-code-comment add-code-comment-right{{if (not $match.CanComment)}} invisible{{end}}" data-path="{{$file.Name}}" data-side="right" data-idx="{{$match.RightIdx}}" data-new-comment-url="{{$.root.Issue.HTMLURL}}/files/reviews/new_comment">{{svg "octicon-plus"}}</a>{{end}}{{if $match.RightHasBIDI}}<a class="ui button code-has-bidi" title="{{$.root.i18n.Tr "repo.diff.has_bidi"}}">&#xfe0f;&#x26a0;</a>{{end}}<code class="code-inner">{{if $match.RightIdx}}{{$section.GetComputedInlineDiffFor $match}}{{end}}</code></td>
 				{{else}}
 					<td class="lines-num lines-num-old" data-line-num="{{if $line.LeftIdx}}{{$line.LeftIdx}}{{end}}"><span rel="{{if $line.LeftIdx}}diff-{{Sha1 $file.Name}}L{{$line.LeftIdx}}{{end}}"></span></td>
 					<td class="lines-type-marker lines-type-marker-old">{{if $line.LeftIdx}}<span class="mono" data-type-marker="{{$line.GetLineTypeMarker}}"></span>{{end}}</td>
-					<td class="lines-code lines-code-old halfwidth">{{if and $.root.SignedUserID $.root.PageIsPullFiles (not (eq .GetType 2))}}<a class="ui primary button add-code-comment add-code-comment-left{{if (not $line.CanComment)}} invisible{{end}}" data-path="{{$file.Name}}" data-side="left" data-idx="{{$line.LeftIdx}}" data-new-comment-url="{{$.root.Issue.HTMLURL}}/files/reviews/new_comment">{{svg "octicon-plus"}}</a>{{end}}<code class="code-inner">{{if $line.LeftIdx}}{{$section.GetComputedInlineDiffFor $line}}{{end}}</code></td>
+					<td class="lines-code lines-code-old halfwidth">{{if and $.root.SignedUserID $.root.PageIsPullFiles (not (eq .GetType 2))}}<a class="ui primary button add-code-comment add-code-comment-left{{if (not $line.CanComment)}} invisible{{end}}" data-path="{{$file.Name}}" data-side="left" data-idx="{{$line.LeftIdx}}" data-new-comment-url="{{$.root.Issue.HTMLURL}}/files/reviews/new_comment">{{svg "octicon-plus"}}</a>{{end}}{{if $line.LeftHasBIDI}}<a class="ui button code-has-bidi" title="{{$.root.i18n.Tr "repo.diff.has_bidi"}}">&#xfe0f;&#x26a0;</a>{{end}}<code class="code-inner">{{if $line.LeftIdx}}{{$section.GetComputedInlineDiffFor $line}}{{end}}</code></td>
 					<td class="lines-num lines-num-new" data-line-num="{{if $line.RightIdx}}{{$line.RightIdx}}{{end}}"><span rel="{{if $line.RightIdx}}diff-{{Sha1 $file.Name}}R{{$line.RightIdx}}{{end}}"></span></td>
 					<td class="lines-type-marker lines-type-marker-new">{{if $line.RightIdx}}<span class="mono" data-type-marker="{{$line.GetLineTypeMarker}}"></span>{{end}}</td>
-					<td class="lines-code lines-code-new halfwidth">{{if and $.root.SignedUserID $.root.PageIsPullFiles (not (eq .GetType 3))}}<a class="ui primary button add-code-comment add-code-comment-right{{if (not $line.CanComment)}} invisible{{end}}" data-path="{{$file.Name}}" data-side="right" data-idx="{{$line.RightIdx}}" data-new-comment-url="{{$.root.Issue.HTMLURL}}/files/reviews/new_comment">{{svg "octicon-plus"}}</a>{{end}}<code class="code-inner">{{if $line.RightIdx}}{{$section.GetComputedInlineDiffFor $line}}{{end}}</code></td>
+					<td class="lines-code lines-code-new halfwidth">{{if and $.root.SignedUserID $.root.PageIsPullFiles (not (eq .GetType 3))}}<a class="ui primary button add-code-comment add-code-comment-right{{if (not $line.CanComment)}} invisible{{end}}" data-path="{{$file.Name}}" data-side="right" data-idx="{{$line.RightIdx}}" data-new-comment-url="{{$.root.Issue.HTMLURL}}/files/reviews/new_comment">{{svg "octicon-plus"}}</a>{{end}}{{if $line.RightHasBIDI}}<a class="ui button code-has-bidi" title="{{$.root.i18n.Tr "repo.diff.has_bidi"}}">&#xfe0f;&#x26a0;</a>{{end}}<code class="code-inner">{{if $line.RightIdx}}{{$section.GetComputedInlineDiffFor $line}}{{end}}</code></td>
 				{{end}}
 			</tr>
 			{{if and (eq .GetType 3) $hasmatch}}

templates/repo/settings/lfs_file.tmpl

@@ -12,6 +12,15 @@
 				</div>
 			</h4>
 			<div class="ui attached table unstackable segment">
+				{{if .HasBIDI}}
+					<div class="ui warning message">
+						<span class="close icon">{{svg "octicon-x" 16 "close inside"}}</span>
+						<div class="header">
+							{{.i18n.Tr "repo.bidi_header"}}
+						</div>
+						{{.i18n.Tr "repo.bidi_description" | Str2html}}
+					</div>
+				{{end}}
 				<div class="file-view{{if .IsMarkup}} markup {{.MarkupType}}{{else if .IsRenderedHTML}} plain-text{{else if .IsTextFile}} code-view{{end}}">
 					{{if .IsMarkup}}
 						{{if .FileContent}}{{.FileContent | Safe}}{{end}}

templates/repo/view_file.tmpl

@@ -64,6 +64,15 @@
 		{{end}}
 	</h4>
 	<div class="ui attached table unstackable segment">
+		{{if .HasBIDI}}
+			<div class="ui error message">
+				<span class="close icon">{{svg "octicon-x" 16 "close inside"}}</span>
+				<div class="header">
+					{{.i18n.Tr "repo.bidi_header"}}
+				</div>
+				{{.i18n.Tr "repo.bidi_description" | Str2html}}
+			</div>
+		{{end}}
 		<div class="file-view{{if .IsMarkup}} markup {{.MarkupType}}{{else if .IsRenderedHTML}} plain-text{{else if .IsTextSource}} code-view{{end}}">
 			{{if .IsMarkup}}
 				{{if .FileContent}}{{.FileContent | Safe}}{{end}}