Skip to content

Commit 0761294

Browse files
bremlbreml
authored
Merge pull request #5 from breml/disallowed-runes-flag
Disallowed runes flag
2 parents 51c3681 + c871de8 commit 0761294

File tree

5 files changed

+194
-24
lines changed

5 files changed

+194
-24
lines changed

‎README.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
# bidichk - checks for dangerous unicode character sequences
22

3+
[![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)
4+
35
bidichk finds dangerous unicode character sequences in Go source files.
46

57
## Considered dangerous unicode characters
@@ -22,3 +24,4 @@ The following unicode characters are considered dangerous:
2224
* [Trojan Source - Proofs-of-Concept](https://github.com/nickboucher/trojan-source)
2325
* [Go proposal: disallow LTR/RTL characters in string literals?](https://github.com/golang/go/issues/20209)
2426
* [cockroachdb/cockroach - PR: lint: add linter for unicode directional formatting characters](https://github.com/cockroachdb/cockroach/pull/72287)
27+
* [Russ Cox - On “Trojan Source” Attacks](https://research.swtch.com/trojan)

‎cmd/bidichk/main.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,5 +7,5 @@ import (
77
)
88

99
func main() {
10-
singlechecker.Main(bidichk.Analyzer)
10+
singlechecker.Main(bidichk.NewAnalyzer())
1111
}

‎pkg/bidichk/bidichk.go

Lines changed: 131 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -2,53 +2,164 @@ package bidichk
22

33
import (
44
"bytes"
5+
"flag"
6+
"fmt"
57
"go/token"
68
"os"
9+
"sort"
710
"strings"
811
"unicode/utf8"
912

1013
"golang.org/x/tools/go/analysis"
1114
)
1215

13-
var Analyzer = &analysis.Analyzer{
14-
Name: "bidichk",
15-
Doc: "Checks for dangerous unicode character sequences",
16-
Run: run,
16+
const (
17+
doc = "bidichk detects dangerous unicode character sequences"
18+
disallowedDoc = `coma separated list of disallowed runes (full name or short name)
19+
20+
Supported runes
21+
22+
LEFT-TO-RIGHT-EMBEDDING, LRE (u+202A)
23+
RIGHT-TO-LEFT-EMBEDDING, RLE (u+202B)
24+
POP-DIRECTIONAL-FORMATTING, PDF (u+202C)
25+
LEFT-TO-RIGHT-OVERRIDE, LRO (u+202D)
26+
RIGHT-TO-LEFT-OVERRIDE, RLO (u+202E)
27+
LEFT-TO-RIGHT-ISOLATE, LRI (u+2066)
28+
RIGHT-TO-LEFT-ISOLATE, RLI (u+2067)
29+
FIRST-STRONG-ISOLATE, FSI (u+2068)
30+
POP-DIRECTIONAL-ISOLATE, PDI (u+2069)
31+
`
32+
)
33+
34+
type disallowedRunes map[string]rune
35+
36+
func (m disallowedRunes) String() string {
37+
ss := make([]string, 0, len(m))
38+
for s := range m {
39+
ss = append(ss, s)
40+
}
41+
sort.Strings(ss)
42+
return strings.Join(ss, ",")
1743
}
1844

19-
func run(pass *analysis.Pass) (interface{}, error) {
45+
func (m disallowedRunes) Set(s string) error {
46+
ss := strings.FieldsFunc(s, func(c rune) bool { return c == ',' })
47+
if len(ss) == 0 {
48+
return nil
49+
}
50+
51+
for k := range m {
52+
delete(m, k)
53+
}
54+
55+
for _, v := range ss {
56+
switch v {
57+
case runeShortNameLRE, runeShortNameRLE, runeShortNamePDF,
58+
runeShortNameLRO, runeShortNameRLO, runeShortNameLRI,
59+
runeShortNameRLI, runeShortNameFSI, runeShortNamePDI:
60+
v = shortNameLookup[v]
61+
fallthrough
62+
case runeNameLRE, runeNameRLE, runeNamePDF,
63+
runeNameLRO, runeNameRLO, runeNameLRI,
64+
runeNameRLI, runeNameFSI, runeNamePDI:
65+
m[v] = runeLookup[v]
66+
default:
67+
return fmt.Errorf("unknown check name %q (see help for full list)", v)
68+
}
69+
}
70+
return nil
71+
}
72+
73+
const (
74+
runeNameLRE = "LEFT-TO-RIGHT-EMBEDDING"
75+
runeNameRLE = "RIGHT-TO-LEFT-EMBEDDING"
76+
runeNamePDF = "POP-DIRECTIONAL-FORMATTING"
77+
runeNameLRO = "LEFT-TO-RIGHT-OVERRIDE"
78+
runeNameRLO = "RIGHT-TO-LEFT-OVERRIDE"
79+
runeNameLRI = "LEFT-TO-RIGHT-ISOLATE"
80+
runeNameRLI = "RIGHT-TO-LEFT-ISOLATE"
81+
runeNameFSI = "FIRST-STRONG-ISOLATE"
82+
runeNamePDI = "POP-DIRECTIONAL-ISOLATE"
83+
84+
runeShortNameLRE = "LRE" // LEFT-TO-RIGHT-EMBEDDING
85+
runeShortNameRLE = "RLE" // RIGHT-TO-LEFT-EMBEDDING
86+
runeShortNamePDF = "PDF" // POP-DIRECTIONAL-FORMATTING
87+
runeShortNameLRO = "LRO" // LEFT-TO-RIGHT-OVERRIDE
88+
runeShortNameRLO = "RLO" // RIGHT-TO-LEFT-OVERRIDE
89+
runeShortNameLRI = "LRI" // LEFT-TO-RIGHT-ISOLATE
90+
runeShortNameRLI = "RLI" // RIGHT-TO-LEFT-ISOLATE
91+
runeShortNameFSI = "FSI" // FIRST-STRONG-ISOLATE
92+
runeShortNamePDI = "PDI" // POP-DIRECTIONAL-ISOLATE
93+
)
94+
95+
var runeLookup = map[string]rune{
96+
runeNameLRE: '\u202A', // LEFT-TO-RIGHT-EMBEDDING
97+
runeNameRLE: '\u202B', // RIGHT-TO-LEFT-EMBEDDING
98+
runeNamePDF: '\u202C', // POP-DIRECTIONAL-FORMATTING
99+
runeNameLRO: '\u202D', // LEFT-TO-RIGHT-OVERRIDE
100+
runeNameRLO: '\u202E', // RIGHT-TO-LEFT-OVERRIDE
101+
runeNameLRI: '\u2066', // LEFT-TO-RIGHT-ISOLATE
102+
runeNameRLI: '\u2067', // RIGHT-TO-LEFT-ISOLATE
103+
runeNameFSI: '\u2068', // FIRST-STRONG-ISOLATE
104+
runeNamePDI: '\u2069', // POP-DIRECTIONAL-ISOLATE
105+
}
106+
107+
var shortNameLookup = map[string]string{
108+
runeShortNameLRE: runeNameLRE,
109+
runeShortNameRLE: runeNameRLE,
110+
runeShortNamePDF: runeNamePDF,
111+
runeShortNameLRO: runeNameLRO,
112+
runeShortNameRLO: runeNameRLO,
113+
runeShortNameLRI: runeNameLRI,
114+
runeShortNameRLI: runeNameRLI,
115+
runeShortNameFSI: runeNameFSI,
116+
runeShortNamePDI: runeNamePDI,
117+
}
118+
119+
type bidichk struct {
120+
disallowedRunes disallowedRunes
121+
}
122+
123+
func NewAnalyzer() *analysis.Analyzer {
124+
bidichk := bidichk{}
125+
bidichk.disallowedRunes = make(map[string]rune, len(runeLookup))
126+
for k, v := range runeLookup {
127+
bidichk.disallowedRunes[k] = v
128+
}
129+
130+
a := &analysis.Analyzer{
131+
Name: "bidichk",
132+
Doc: doc,
133+
Run: bidichk.run,
134+
}
135+
136+
a.Flags.Init("bidichk", flag.ExitOnError)
137+
a.Flags.Var(&bidichk.disallowedRunes, "disallowed-runes", disallowedDoc)
138+
139+
return a
140+
}
141+
142+
func (b bidichk) run(pass *analysis.Pass) (interface{}, error) {
20143
var err error
21144

22145
pass.Fset.Iterate(func(f *token.File) bool {
23146
if strings.HasPrefix(f.Name(), "$GOROOT") {
24147
return true
25148
}
26149

27-
return check(f.Name(), f.Pos(0), pass) == nil
150+
return b.check(f.Name(), f.Pos(0), pass) == nil
28151
})
29152

30153
return nil, err
31154
}
32155

33-
var disallowedRunes = map[string]rune{
34-
"LEFT-TO-RIGHT-EMBEDDING": '\u202A',
35-
"RIGHT-TO-LEFT-EMBEDDING": '\u202B',
36-
"POP-DIRECTIONAL-FORMATTING": '\u202C',
37-
"LEFT-TO-RIGHT-OVERRIDE": '\u202D',
38-
"RIGHT-TO-LEFT-OVERRIDE": '\u202E',
39-
"LEFT-TO-RIGHT-ISOLATE": '\u2066',
40-
"RIGHT-TO-LEFT-ISOLATE": '\u2067',
41-
"FIRST-STRONG-ISOLATE": '\u2068',
42-
"POP-DIRECTIONAL-ISOLATE": '\u2069',
43-
}
44-
45-
func check(filename string, pos token.Pos, pass *analysis.Pass) error {
156+
func (b bidichk) check(filename string, pos token.Pos, pass *analysis.Pass) error {
46157
body, err := os.ReadFile(filename)
47158
if err != nil {
48159
return err
49160
}
50161

51-
for name, r := range disallowedRunes {
162+
for name, r := range b.disallowedRunes {
52163
start := 0
53164
for {
54165
idx := bytes.IndexRune(body[start:], r)

‎pkg/bidichk/bidichk_test.go

Lines changed: 47 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,19 +5,63 @@ import (
55
"path/filepath"
66
"testing"
77

8+
"golang.org/x/tools/go/analysis"
89
"golang.org/x/tools/go/analysis/analysistest"
910

1011
"github.com/breml/bidichk/pkg/bidichk"
1112
)
1213

1314
func TestAll(t *testing.T) {
15+
tt := []struct {
16+
name string
17+
analyzerFunc func() *analysis.Analyzer
18+
testdataDir string
19+
}{
20+
{
21+
name: "simple",
22+
analyzerFunc: bidichk.NewAnalyzer,
23+
testdataDir: "simple",
24+
},
25+
{
26+
name: "commenting-out",
27+
analyzerFunc: bidichk.NewAnalyzer,
28+
testdataDir: "commenting-out",
29+
},
30+
{
31+
name: "stretched-string",
32+
analyzerFunc: bidichk.NewAnalyzer,
33+
testdataDir: "stretched-string",
34+
},
35+
{
36+
name: "commenting-out-lri-only",
37+
analyzerFunc: func() *analysis.Analyzer {
38+
a := bidichk.NewAnalyzer()
39+
_ = a.Flags.Set("disallowed-runes", "LEFT-TO-RIGHT-ISOLATE")
40+
return a
41+
},
42+
testdataDir: "commenting-out-lri-only",
43+
},
44+
{
45+
name: "commenting-out-lri-only short name",
46+
analyzerFunc: func() *analysis.Analyzer {
47+
a := bidichk.NewAnalyzer()
48+
_ = a.Flags.Set("disallowed-runes", "LRI")
49+
return a
50+
},
51+
testdataDir: "commenting-out-lri-only",
52+
},
53+
}
54+
1455
wd, err := os.Getwd()
1556
if err != nil {
1657
t.Fatalf("Failed to get wd: %s", err)
1758
}
1859

1960
testdata := filepath.Join(filepath.Dir(filepath.Dir(wd)), "testdata")
20-
analysistest.Run(t, testdata, bidichk.Analyzer, "simple")
21-
analysistest.Run(t, testdata, bidichk.Analyzer, "commenting-out")
22-
analysistest.Run(t, testdata, bidichk.Analyzer, "stretched-string")
61+
62+
for _, tc := range tt {
63+
t.Run(tc.name, func(t *testing.T) {
64+
analysistest.Run(t, testdata, tc.analyzerFunc(), tc.testdataDir)
65+
})
66+
}
2367
}

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
package main
2+
3+
import "fmt"
4+
5+
func main() {
6+
isAdmin := false
7+
isSuperAdmin := false
8+
isAdmin = isAdmin || isSuperAdmin
9+
/*‮ } ⁦if (isAdmin)⁩ ⁦ begin admins only */ // want "found dangerous unicode character sequence LEFT-TO-RIGHT-ISOLATE" "found dangerous unicode character sequence LEFT-TO-RIGHT-ISOLATE"
10+
fmt.Println("You are an admin.")
11+
/* end admins only ‮ { ⁦*/ // want "found dangerous unicode character sequence LEFT-TO-RIGHT-ISOLATE"
12+
}

0 commit comments

Comments
 (0)