diff --git a/http2/ascii.go b/http2/ascii.go index 0c58d727..17caa205 100644 --- a/http2/ascii.go +++ b/http2/ascii.go @@ -6,6 +6,10 @@ package http2 import "strings" +// The HTTP protocols are defined in terms of ASCII, not Unicode. This file +// contains helper functions which may use Unicode-aware functions which would +// otherwise be unsafe and could introduce vulnerabilities if used improperly. + // asciiEqualFold is strings.EqualFold, ASCII only. It reports whether s and t // are equal, ASCII-case-insensitively. func asciiEqualFold(s, t string) bool { diff --git a/http2/http2_test.go b/http2/http2_test.go index d6007bec..f77c08a1 100644 --- a/http2/http2_test.go +++ b/http2/http2_test.go @@ -9,8 +9,12 @@ import ( "errors" "flag" "fmt" + "io/ioutil" "net/http" + "os" "os/exec" + "path/filepath" + "regexp" "strconv" "strings" "testing" @@ -288,3 +292,57 @@ func TestConfigureServerIdleTimeout_Go18(t *testing.T) { } } } + +var forbiddenStringsFunctions = map[string]bool{ + // Functions that use Unicode-aware case folding. + "EqualFold": true, + "Title": true, + "ToLower": true, + "ToLowerSpecial": true, + "ToTitle": true, + "ToTitleSpecial": true, + "ToUpper": true, + "ToUpperSpecial": true, + + // Functions that use Unicode-aware spaces. + "Fields": true, + "TrimSpace": true, +} + +// TestNoUnicodeStrings checks that nothing in net/http uses the Unicode-aware +// strings and bytes package functions. HTTP is mostly ASCII based, and doing +// Unicode-aware case folding or space stripping can introduce vulnerabilities. +func TestNoUnicodeStrings(t *testing.T) { + re := regexp.MustCompile(`(strings|bytes).([A-Za-z]+)`) + if err := filepath.Walk(".", func(path string, info os.FileInfo, err error) error { + if err != nil { + t.Fatal(err) + } + + if path == "h2i" || path == "h2c" { + return filepath.SkipDir + } + if !strings.HasSuffix(path, ".go") || + strings.HasSuffix(path, "_test.go") || + path == "ascii.go" || info.IsDir() { + return nil + } + + contents, err := ioutil.ReadFile(path) + if err != nil { + t.Fatal(err) + } + for lineNum, line := range strings.Split(string(contents), "\n") { + for _, match := range re.FindAllStringSubmatch(line, -1) { + if !forbiddenStringsFunctions[match[2]] { + continue + } + t.Errorf("disallowed call to %s at %s:%d", match[0], path, lineNum+1) + } + } + + return nil + }); err != nil { + t.Fatal(err) + } +}