From 84b48f89b13b8e913b7e836374eaa06359af2575 Mon Sep 17 00:00:00 2001 From: Filippo Valsorda Date: Mon, 10 May 2021 19:41:49 +0200 Subject: [PATCH] http2: check that Unicode-aware functions are not used Change-Id: I366ba1f21c48773fa923e47501ccd7efb8c99a2d Reviewed-on: https://go-review.googlesource.com/c/net/+/318430 Run-TryBot: Filippo Valsorda TryBot-Result: Go Bot Trust: Filippo Valsorda Trust: Katie Hockman Reviewed-by: Katie Hockman --- http2/ascii.go | 4 ++++ http2/http2_test.go | 58 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+) 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) + } +}