Merge pull request #5 from breml/disallowed-runes-flag

Disallowed runes flag

README.md

@@ -1,5 +1,7 @@
 # bidichk - checks for dangerous unicode character sequences
 
+[![Test Status](https://github.com/breml/bidichk/workflows/Go%20Matrix/badge.svg)](https://github.com/breml/logstash-config/actions?query=workflow%3AGo%20Matrix) [![License](https://img.shields.io/badge/license-MIT-blue.svg)](LICENSE)
+
 bidichk finds dangerous unicode character sequences in Go source files.
 
 ## Considered dangerous unicode characters
@@ -22,3 +24,4 @@ The following unicode characters are considered dangerous:
 * [Trojan Source - Proofs-of-Concept](https://github.com/nickboucher/trojan-source)
 * [Go proposal: disallow LTR/RTL characters in string literals?](https://github.com/golang/go/issues/20209)
 * [cockroachdb/cockroach - PR: lint: add linter for unicode directional formatting characters](https://github.com/cockroachdb/cockroach/pull/72287)
+* [Russ Cox - On “Trojan Source” Attacks](https://research.swtch.com/trojan)

cmd/bidichk/main.go

@@ -7,5 +7,5 @@ import (
 )
 
 func main() {
-	singlechecker.Main(bidichk.Analyzer)
+	singlechecker.Main(bidichk.NewAnalyzer())
 }

pkg/bidichk/bidichk.go

@@ -2,53 +2,164 @@ package bidichk
 
 import (
 	"bytes"
+	"flag"
+	"fmt"
 	"go/token"
 	"os"
+	"sort"
 	"strings"
 	"unicode/utf8"
 
 	"golang.org/x/tools/go/analysis"
 )
 
-var Analyzer = &analysis.Analyzer{
-	Name: "bidichk",
-	Doc:  "Checks for dangerous unicode character sequences",
-	Run:  run,
+const (
+	doc           = "bidichk detects dangerous unicode character sequences"
+	disallowedDoc = `coma separated list of disallowed runes (full name or short name)
+
+Supported runes
+
+LEFT-TO-RIGHT-EMBEDDING, LRE (u+202A)
+RIGHT-TO-LEFT-EMBEDDING, RLE (u+202B)
+POP-DIRECTIONAL-FORMATTING, PDF (u+202C)
+LEFT-TO-RIGHT-OVERRIDE, LRO (u+202D)
+RIGHT-TO-LEFT-OVERRIDE, RLO (u+202E)
+LEFT-TO-RIGHT-ISOLATE, LRI (u+2066)
+RIGHT-TO-LEFT-ISOLATE, RLI (u+2067)
+FIRST-STRONG-ISOLATE, FSI (u+2068)
+POP-DIRECTIONAL-ISOLATE, PDI (u+2069)
+`
+)
+
+type disallowedRunes map[string]rune
+
+func (m disallowedRunes) String() string {
+	ss := make([]string, 0, len(m))
+	for s := range m {
+		ss = append(ss, s)
+	}
+	sort.Strings(ss)
+	return strings.Join(ss, ",")
 }
 
-func run(pass *analysis.Pass) (interface{}, error) {
+func (m disallowedRunes) Set(s string) error {
+	ss := strings.FieldsFunc(s, func(c rune) bool { return c == ',' })
+	if len(ss) == 0 {
+		return nil
+	}
+
+	for k := range m {
+		delete(m, k)
+	}
+
+	for _, v := range ss {
+		switch v {
+		case runeShortNameLRE, runeShortNameRLE, runeShortNamePDF,
+			runeShortNameLRO, runeShortNameRLO, runeShortNameLRI,
+			runeShortNameRLI, runeShortNameFSI, runeShortNamePDI:
+			v = shortNameLookup[v]
+			fallthrough
+		case runeNameLRE, runeNameRLE, runeNamePDF,
+			runeNameLRO, runeNameRLO, runeNameLRI,
+			runeNameRLI, runeNameFSI, runeNamePDI:
+			m[v] = runeLookup[v]
+		default:
+			return fmt.Errorf("unknown check name %q (see help for full list)", v)
+		}
+	}
+	return nil
+}
+
+const (
+	runeNameLRE = "LEFT-TO-RIGHT-EMBEDDING"
+	runeNameRLE = "RIGHT-TO-LEFT-EMBEDDING"
+	runeNamePDF = "POP-DIRECTIONAL-FORMATTING"
+	runeNameLRO = "LEFT-TO-RIGHT-OVERRIDE"
+	runeNameRLO = "RIGHT-TO-LEFT-OVERRIDE"
+	runeNameLRI = "LEFT-TO-RIGHT-ISOLATE"
+	runeNameRLI = "RIGHT-TO-LEFT-ISOLATE"
+	runeNameFSI = "FIRST-STRONG-ISOLATE"
+	runeNamePDI = "POP-DIRECTIONAL-ISOLATE"
+
+	runeShortNameLRE = "LRE" // LEFT-TO-RIGHT-EMBEDDING
+	runeShortNameRLE = "RLE" // RIGHT-TO-LEFT-EMBEDDING
+	runeShortNamePDF = "PDF" // POP-DIRECTIONAL-FORMATTING
+	runeShortNameLRO = "LRO" // LEFT-TO-RIGHT-OVERRIDE
+	runeShortNameRLO = "RLO" // RIGHT-TO-LEFT-OVERRIDE
+	runeShortNameLRI = "LRI" // LEFT-TO-RIGHT-ISOLATE
+	runeShortNameRLI = "RLI" // RIGHT-TO-LEFT-ISOLATE
+	runeShortNameFSI = "FSI" // FIRST-STRONG-ISOLATE
+	runeShortNamePDI = "PDI" // POP-DIRECTIONAL-ISOLATE
+)
+
+var runeLookup = map[string]rune{
+	runeNameLRE: '\u202A', // LEFT-TO-RIGHT-EMBEDDING
+	runeNameRLE: '\u202B', // RIGHT-TO-LEFT-EMBEDDING
+	runeNamePDF: '\u202C', // POP-DIRECTIONAL-FORMATTING
+	runeNameLRO: '\u202D', // LEFT-TO-RIGHT-OVERRIDE
+	runeNameRLO: '\u202E', // RIGHT-TO-LEFT-OVERRIDE
+	runeNameLRI: '\u2066', // LEFT-TO-RIGHT-ISOLATE
+	runeNameRLI: '\u2067', // RIGHT-TO-LEFT-ISOLATE
+	runeNameFSI: '\u2068', // FIRST-STRONG-ISOLATE
+	runeNamePDI: '\u2069', // POP-DIRECTIONAL-ISOLATE
+}
+
+var shortNameLookup = map[string]string{
+	runeShortNameLRE: runeNameLRE,
+	runeShortNameRLE: runeNameRLE,
+	runeShortNamePDF: runeNamePDF,
+	runeShortNameLRO: runeNameLRO,
+	runeShortNameRLO: runeNameRLO,
+	runeShortNameLRI: runeNameLRI,
+	runeShortNameRLI: runeNameRLI,
+	runeShortNameFSI: runeNameFSI,
+	runeShortNamePDI: runeNamePDI,
+}
+
+type bidichk struct {
+	disallowedRunes disallowedRunes
+}
+
+func NewAnalyzer() *analysis.Analyzer {
+	bidichk := bidichk{}
+	bidichk.disallowedRunes = make(map[string]rune, len(runeLookup))
+	for k, v := range runeLookup {
+		bidichk.disallowedRunes[k] = v
+	}
+
+	a := &analysis.Analyzer{
+		Name: "bidichk",
+		Doc:  doc,
+		Run:  bidichk.run,
+	}
+
+	a.Flags.Init("bidichk", flag.ExitOnError)
+	a.Flags.Var(&bidichk.disallowedRunes, "disallowed-runes", disallowedDoc)
+
+	return a
+}
+
+func (b bidichk) run(pass *analysis.Pass) (interface{}, error) {
 	var err error
 
 	pass.Fset.Iterate(func(f *token.File) bool {
 		if strings.HasPrefix(f.Name(), "$GOROOT") {
 			return true
 		}
 
-		return check(f.Name(), f.Pos(0), pass) == nil
+		return b.check(f.Name(), f.Pos(0), pass) == nil
 	})
 
 	return nil, err
 }
 
-var disallowedRunes = map[string]rune{
-	"LEFT-TO-RIGHT-EMBEDDING":    '\u202A',
-	"RIGHT-TO-LEFT-EMBEDDING":    '\u202B',
-	"POP-DIRECTIONAL-FORMATTING": '\u202C',
-	"LEFT-TO-RIGHT-OVERRIDE":     '\u202D',
-	"RIGHT-TO-LEFT-OVERRIDE":     '\u202E',
-	"LEFT-TO-RIGHT-ISOLATE":      '\u2066',
-	"RIGHT-TO-LEFT-ISOLATE":      '\u2067',
-	"FIRST-STRONG-ISOLATE":       '\u2068',
-	"POP-DIRECTIONAL-ISOLATE":    '\u2069',
-}
-
-func check(filename string, pos token.Pos, pass *analysis.Pass) error {
+func (b bidichk) check(filename string, pos token.Pos, pass *analysis.Pass) error {
 	body, err := os.ReadFile(filename)
 	if err != nil {
 		return err
 	}
 
-	for name, r := range disallowedRunes {
+	for name, r := range b.disallowedRunes {
 		start := 0
 		for {
 			idx := bytes.IndexRune(body[start:], r)

pkg/bidichk/bidichk_test.go

@@ -5,19 +5,63 @@ import (
 	"path/filepath"
 	"testing"
 
+	"golang.org/x/tools/go/analysis"
 	"golang.org/x/tools/go/analysis/analysistest"
 
 	"github.com/breml/bidichk/pkg/bidichk"
 )
 
 func TestAll(t *testing.T) {
+	tt := []struct {
+		name         string
+		analyzerFunc func() *analysis.Analyzer
+		testdataDir  string
+	}{
+		{
+			name:         "simple",
+			analyzerFunc: bidichk.NewAnalyzer,
+			testdataDir:  "simple",
+		},
+		{
+			name:         "commenting-out",
+			analyzerFunc: bidichk.NewAnalyzer,
+			testdataDir:  "commenting-out",
+		},
+		{
+			name:         "stretched-string",
+			analyzerFunc: bidichk.NewAnalyzer,
+			testdataDir:  "stretched-string",
+		},
+		{
+			name: "commenting-out-lri-only",
+			analyzerFunc: func() *analysis.Analyzer {
+				a := bidichk.NewAnalyzer()
+				_ = a.Flags.Set("disallowed-runes", "LEFT-TO-RIGHT-ISOLATE")
+				return a
+			},
+			testdataDir: "commenting-out-lri-only",
+		},
+		{
+			name: "commenting-out-lri-only short name",
+			analyzerFunc: func() *analysis.Analyzer {
+				a := bidichk.NewAnalyzer()
+				_ = a.Flags.Set("disallowed-runes", "LRI")
+				return a
+			},
+			testdataDir: "commenting-out-lri-only",
+		},
+	}
+
 	wd, err := os.Getwd()
 	if err != nil {
 		t.Fatalf("Failed to get wd: %s", err)
 	}
 
 	testdata := filepath.Join(filepath.Dir(filepath.Dir(wd)), "testdata")
-	analysistest.Run(t, testdata, bidichk.Analyzer, "simple")
-	analysistest.Run(t, testdata, bidichk.Analyzer, "commenting-out")
-	analysistest.Run(t, testdata, bidichk.Analyzer, "stretched-string")
+
+	for _, tc := range tt {
+		t.Run(tc.name, func(t *testing.T) {
+			analysistest.Run(t, testdata, tc.analyzerFunc(), tc.testdataDir)
+		})
+	}
 }

testdata/src/commenting-out-lri-only/commenting-out-lri-only.go

@@ -0,0 +1,12 @@
+package main
+
+import "fmt"
+
+func main() {
+	isAdmin := false
+	isSuperAdmin := false
+	isAdmin = isAdmin || isSuperAdmin
+	/*‮ } ⁦if (isAdmin)⁩ ⁦ begin admins only */ // want "found dangerous unicode character sequence LEFT-TO-RIGHT-ISOLATE" "found dangerous unicode character sequence LEFT-TO-RIGHT-ISOLATE"
+	fmt.Println("You are an admin.")
+	/* end admins only ‮ { ⁦*/ // want "found dangerous unicode character sequence LEFT-TO-RIGHT-ISOLATE"
+}