Prevent Usage of Stdlib File/Dir Writing With Static Analysis (#7685)

* write file and mkdirall analyzers

* include analyzer in build bazel

* comments to the single entrypoint and fix validator references

* enforce 600 for files, 700 for dirs

* pass validator tests

* add to nogo

* remove references

* beaconfuzz

* docker img

* fix up kv issue

* mkdir if not exists

* radek comments

* final comments

* Try to fix file problem

Co-authored-by: Ivan Martinez <ivanthegreatdev@gmail.com>

tools/analyzers/properpermissions/BUILD.bazel

@@ -0,0 +1,28 @@
+load("@prysm//tools/go:def.bzl", "go_library")
+load("@io_bazel_rules_go//go:def.bzl", "go_tool_library")
+
+go_library(
+    name = "go_default_library",
+    srcs = ["analyzer.go"],
+    importpath = "github.com/prysmaticlabs/prysm/tools/analyzers/properpermissions",
+    visibility = ["//visibility:public"],
+    deps = [
+        "@org_golang_x_tools//go/analysis:go_default_library",
+        "@org_golang_x_tools//go/analysis/passes/inspect:go_default_library",
+        "@org_golang_x_tools//go/ast/inspector:go_default_library",
+    ],
+)
+
+go_tool_library(
+    name = "go_tool_library",
+    srcs = ["analyzer.go"],
+    importpath = "github.com/prysmaticlabs/prysm/tools/analyzers/properpermissions",
+    visibility = ["//visibility:public"],
+    deps = [
+        "@org_golang_x_tools//go/analysis:go_tool_library",
+        "@org_golang_x_tools//go/analysis/passes/inspect:go_tool_library",
+        "@org_golang_x_tools//go/ast/inspector:go_tool_library",
+    ],
+)
+
+# gazelle:exclude analyzer_test.go

tools/analyzers/properpermissions/analyzer.go

@@ -0,0 +1,104 @@
+// Package properpermissions implements a static analyzer to ensure that Prysm does not
+// use ioutil.MkdirAll or os.WriteFile as they are unsafe when it comes to guaranteeing
+// file permissions and not overriding existing permissions. Instead, users are warned
+// to utilize shared/fileutil as the canonical solution.
+package properpermissions
+
+import (
+	"errors"
+	"fmt"
+	"go/ast"
+
+	"golang.org/x/tools/go/analysis"
+	"golang.org/x/tools/go/analysis/passes/inspect"
+	"golang.org/x/tools/go/ast/inspector"
+)
+
+// Doc explaining the tool.
+const Doc = "Tool to enforce usage of Prysm's internal file-writing utils instead of os.MkdirAll or ioutil.WriteFile"
+
+var (
+	errUnsafePackage = errors.New(
+		"os and ioutil dir and file writing functions are not permissions-safe, use shared/fileutil",
+	)
+	disallowedFns = []string{"MkdirAll", "WriteFile"}
+)
+
+// Analyzer runs static analysis.
+var Analyzer = &analysis.Analyzer{
+	Name:     "properpermissions",
+	Doc:      Doc,
+	Requires: []*analysis.Analyzer{inspect.Analyzer},
+	Run:      run,
+}
+
+func run(pass *analysis.Pass) (interface{}, error) {
+	inspect, ok := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
+	if !ok {
+		return nil, errors.New("analyzer is not type *inspector.Inspector")
+	}
+
+	nodeFilter := []ast.Node{
+		(*ast.File)(nil),
+		(*ast.ImportSpec)(nil),
+		(*ast.CallExpr)(nil),
+	}
+
+	aliases := make(map[string]string)
+
+	inspect.Preorder(nodeFilter, func(node ast.Node) {
+		switch stmt := node.(type) {
+		case *ast.File:
+			// Reset aliases (per file).
+			aliases = make(map[string]string)
+		case *ast.ImportSpec:
+			// Collect aliases.
+			pkg := stmt.Path.Value
+			if pkg == "\"os\"" {
+				if stmt.Name != nil {
+					aliases[stmt.Name.Name] = pkg
+				} else {
+					aliases["os"] = pkg
+				}
+			}
+			if pkg == "\"io/ioutil\"" {
+				if stmt.Name != nil {
+					aliases[stmt.Name.Name] = pkg
+				} else {
+					aliases["ioutil"] = pkg
+				}
+			}
+		case *ast.CallExpr:
+			// Check if any of disallowed functions have been used.
+			for alias, pkg := range aliases {
+				for _, fn := range disallowedFns {
+					if isPkgDot(stmt.Fun, alias, fn) {
+						pass.Reportf(
+							node.Pos(),
+							fmt.Sprintf(
+								"%v: %s.%s() (from %s)",
+								errUnsafePackage,
+								alias,
+								fn,
+								pkg,
+							),
+						)
+					}
+				}
+			}
+		}
+	})
+
+	return nil, nil
+}
+
+func isPkgDot(expr ast.Expr, pkg, name string) bool {
+	sel, ok := expr.(*ast.SelectorExpr)
+	res := ok && isIdent(sel.X, pkg) && isIdent(sel.Sel, name)
+	return res
+}
+
+func isIdent(expr ast.Expr, ident string) bool {
+	id, ok := expr.(*ast.Ident)
+	return ok && id.Name == ident
+}

tools/analyzers/properpermissions/analyzer_test.go

@@ -0,0 +1,11 @@
+package properpermissions
+
+import (
+	"testing"
+
+	"golang.org/x/tools/go/analysis/analysistest"
+)
+
+func TestAnalyzer(t *testing.T) {
+	analysistest.Run(t, analysistest.TestData(), Analyzer)
+}

tools/analyzers/properpermissions/testdata/BUILD.bazel

@@ -0,0 +1,11 @@
+load("@prysm//tools/go:def.bzl", "go_library")
+
+go_library(
+    name = "go_default_library",
+    srcs = [
+        "custom_imports.go",
+        "regular_imports.go",
+    ],
+    importpath = "github.com/prysmaticlabs/prysm/tools/analyzers/properpermissions/testdata",
+    visibility = ["//visibility:public"],
+)

tools/analyzers/properpermissions/testdata/custom_imports.go

@@ -0,0 +1,18 @@
+package testdata
+
+import (
+	"crypto/rand"
+	"fmt"
+	ioAlias "io/ioutil"
+	"math/big"
+	osAlias "os"
+	"path/filepath"
+)
+
+func UseAliasedPackages() {
+	randPath, _ := rand.Int(rand.Reader, big.NewInt(1000000))
+	p := filepath.Join(tempDir(), fmt.Sprintf("/%d", randPath))
+	_ = osAlias.MkdirAll(p, osAlias.ModePerm) // want "os and ioutil dir and file writing functions are not permissions-safe, use shared/fileutil"
+	someFile := filepath.Join(p, "some.txt")
+	_ = ioAlias.WriteFile(someFile, []byte("hello"), osAlias.ModePerm) // want "os and ioutil dir and file writing functions are not permissions-safe, use shared/fileutil"
+}

tools/analyzers/properpermissions/testdata/regular_imports.go

@@ -0,0 +1,26 @@
+package testdata
+
+import (
+	"crypto/rand"
+	"fmt"
+	"io/ioutil"
+	"math/big"
+	"os"
+	"path/filepath"
+)
+
+func tempDir() string {
+	d := os.Getenv("TEST_TMPDIR")
+	if d == "" {
+		return os.TempDir()
+	}
+	return d
+}
+
+func UseOsMkdirAllAndWriteFile() {
+	randPath, _ := rand.Int(rand.Reader, big.NewInt(1000000))
+	p := filepath.Join(tempDir(), fmt.Sprintf("/%d", randPath))
+	_ = os.MkdirAll(p, os.ModePerm) // want "os and ioutil dir and file writing functions are not permissions-safe, use shared/fileutil"
+	someFile := filepath.Join(p, "some.txt")
+	_ = ioutil.WriteFile(someFile, []byte("hello"), os.ModePerm) // want "os and ioutil dir and file writing functions are not permissions-safe, use shared/fileutil"
+}