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>

BUILD.bazel

@@ -112,6 +112,7 @@ nogo(
         "//tools/analyzers/nop:go_tool_library",
         "//tools/analyzers/slicedirect:go_tool_library",
         "//tools/analyzers/ineffassign:go_tool_library",
+        "//tools/analyzers/properpermissions:go_tool_library",
     ] + select({
         # nogo checks that fail with coverage enabled.
         ":coverage_enabled": [],

beacon-chain/core/state/interop/BUILD.bazel

@@ -15,6 +15,7 @@ go_library(
     deps = [
         "//beacon-chain/state:go_default_library",
         "//shared/featureconfig:go_default_library",
+        "//shared/fileutil:go_default_library",
         "@com_github_prysmaticlabs_ethereumapis//eth/v1alpha1:go_default_library",
         "@com_github_sirupsen_logrus//:go_default_library",
     ],

beacon-chain/core/state/interop/write_block_to_disk.go

@@ -2,12 +2,12 @@ package interop
 
 import (
 	"fmt"
-	"io/ioutil"
 	"os"
 	"path"
 
 	ethpb "github.com/prysmaticlabs/ethereumapis/eth/v1alpha1"
 	"github.com/prysmaticlabs/prysm/shared/featureconfig"
+	"github.com/prysmaticlabs/prysm/shared/fileutil"
 )
 
 // WriteBlockToDisk as a block ssz. Writes to temp directory. Debug!
@@ -27,7 +27,7 @@ func WriteBlockToDisk(block *ethpb.SignedBeaconBlock, failed bool) {
 		log.WithError(err).Error("Failed to ssz encode block")
 		return
 	}
-	if err := ioutil.WriteFile(fp, enc, 0664); err != nil {
+	if err := fileutil.WriteFile(fp, enc); err != nil {
 		log.WithError(err).Error("Failed to write to disk")
 	}
 }

beacon-chain/core/state/interop/write_state_to_disk.go

@@ -2,12 +2,12 @@ package interop
 
 import (
 	"fmt"
-	"io/ioutil"
 	"os"
 	"path"
 
 	stateTrie "github.com/prysmaticlabs/prysm/beacon-chain/state"
 	"github.com/prysmaticlabs/prysm/shared/featureconfig"
+	"github.com/prysmaticlabs/prysm/shared/fileutil"
 )
 
 // WriteStateToDisk as a state ssz. Writes to temp directory. Debug!
@@ -22,7 +22,7 @@ func WriteStateToDisk(state *stateTrie.BeaconState) {
 		log.WithError(err).Error("Failed to ssz encode state")
 		return
 	}
-	if err := ioutil.WriteFile(fp, enc, 0664); err != nil {
+	if err := fileutil.WriteFile(fp, enc); err != nil {
 		log.WithError(err).Error("Failed to write to disk")
 	}
 }

beacon-chain/db/kv/backup.go

@@ -3,7 +3,6 @@ package kv
 import (
 	"context"
 	"fmt"
-	"os"
 	"path"
 
 	"github.com/pkg/errors"
@@ -40,7 +39,7 @@ func (s *Store) Backup(ctx context.Context, outputDir string) error {
 		return errors.New("no head block")
 	}
 	// Ensure the backups directory exists.
-	if err := os.MkdirAll(backupsDir, params.BeaconIoConfig().ReadWriteExecutePermissions); err != nil {
+	if err := fileutil.MkdirAll(backupsDir); err != nil {
 		return err
 	}
 	backupPath := path.Join(backupsDir, fmt.Sprintf("prysm_beacondb_at_slot_%07d.backup", head.Block.Slot))

beacon-chain/db/kv/kv.go

@@ -13,6 +13,7 @@ import (
 	prombolt "github.com/prysmaticlabs/prombbolt"
 	"github.com/prysmaticlabs/prysm/beacon-chain/cache"
 	"github.com/prysmaticlabs/prysm/beacon-chain/db/iface"
+	"github.com/prysmaticlabs/prysm/shared/fileutil"
 	"github.com/prysmaticlabs/prysm/shared/params"
 	bolt "go.etcd.io/bbolt"
 )
@@ -46,9 +47,15 @@ type Store struct {
 // path specified, creates the kv-buckets based on the schema, and stores
 // an open connection db object as a property of the Store struct.
 func NewKVStore(dirPath string, stateSummaryCache *cache.StateSummaryCache) (*Store, error) {
-	if err := os.MkdirAll(dirPath, params.BeaconIoConfig().ReadWriteExecutePermissions); err != nil {
+	hasDir, err := fileutil.HasDir(dirPath)
+	if err != nil {
 		return nil, err
 	}
+	if !hasDir {
+		if err := fileutil.MkdirAll(dirPath); err != nil {
+			return nil, err
+		}
+	}
 	datafile := path.Join(dirPath, databaseFileName)
 	boltDB, err := bolt.Open(datafile, params.BeaconIoConfig().ReadWritePermissions, &bolt.Options{Timeout: 1 * time.Second, InitialMmapSize: 10e6})
 	if err != nil {

beacon-chain/p2p/BUILD.bazel

@@ -48,6 +48,7 @@ go_library(
         "//beacon-chain/p2p/types:go_default_library",
         "//proto/beacon/p2p/v1:go_default_library",
         "//shared:go_default_library",
+        "//shared/fileutil:go_default_library",
         "//shared/hashutil:go_default_library",
         "//shared/iputils:go_default_library",
         "//shared/p2putils:go_default_library",

beacon-chain/p2p/utils.go

@@ -19,8 +19,8 @@ import (
 	"github.com/pkg/errors"
 	"github.com/prysmaticlabs/go-bitfield"
 	pbp2p "github.com/prysmaticlabs/prysm/proto/beacon/p2p/v1"
+	"github.com/prysmaticlabs/prysm/shared/fileutil"
 	"github.com/prysmaticlabs/prysm/shared/iputils"
-	"github.com/prysmaticlabs/prysm/shared/params"
 	"github.com/sirupsen/logrus"
 )
 
@@ -78,7 +78,7 @@ func privKey(cfg *Config) (*ecdsa.PrivateKey, error) {
 		}
 		dst := make([]byte, hex.EncodedLen(len(rawbytes)))
 		hex.Encode(dst, rawbytes)
-		if err = ioutil.WriteFile(defaultKeyPath, dst, params.BeaconIoConfig().ReadWritePermissions); err != nil {
+		if err = fileutil.WriteFile(defaultKeyPath, dst); err != nil {
 			return nil, err
 		}
 		convertedKey := convertFromInterfacePrivKey(priv)
@@ -129,7 +129,7 @@ func metaDataFromConfig(cfg *Config) (*pbp2p.MetaData, error) {
 		if err != nil {
 			return nil, err
 		}
-		if err = ioutil.WriteFile(defaultKeyPath, dst, params.BeaconIoConfig().ReadWritePermissions); err != nil {
+		if err = fileutil.WriteFile(defaultKeyPath, dst); err != nil {
 			return nil, err
 		}
 		return metaData, nil

endtoend/helpers/helpers.go

@@ -133,9 +133,6 @@ func LogErrorOutput(t *testing.T, file io.Reader, title string, index int) {
 
 // WritePprofFiles writes the memory heap and cpu profile files to the test path.
 func WritePprofFiles(testDir string, index int) error {
-	if err := os.MkdirAll(filepath.Join(testDir), os.ModePerm); err != nil {
-		return err
-	}
 	url := fmt.Sprintf("http://127.0.0.1:%d/debug/pprof/heap", e2e.TestParams.BeaconNodeRPCPort+50+index)
 	filePath := filepath.Join(testDir, fmt.Sprintf(memoryHeapFileName, index))
 	if err := writeURLRespAtPath(url, filePath); err != nil {

endtoend/params/params.go

@@ -6,7 +6,7 @@ import (
 	"errors"
 	"fmt"
 	"os"
-	"path"
+	"path/filepath"
 	"strconv"
 
 	"github.com/bazelbuild/rules_go/go/tools/bazel"
@@ -66,9 +66,10 @@ func Init(beaconNodeCount int) error {
 	if err != nil {
 		return err
 	}
+	testPath = filepath.Join(testPath, fmt.Sprintf("shard-%d", testIndex))
 
 	TestParams = &params{
-		TestPath:              path.Join(testPath, fmt.Sprintf("shard-%d", testIndex)),
+		TestPath:              testPath,
 		LogPath:               logPath,
 		TestShardIndex:        testIndex,
 		BeaconNodeCount:       beaconNodeCount,

nogo_config.json

@@ -74,7 +74,8 @@
       "shared/mock/.*\\.go": "Mocks are OK",
       ".*/.*mock\\.go": "Mocks are OK",
       ".*/testmain\\.go": "Test runner generated code",
-      "proto/.*": "Generated protobuf related code"
+      "proto/.*": "Generated protobuf related code",
+      "tools/analyzers/properpermissions/testdata/.*": "Analyzer breaks rules"
     }
   },
   "featureconfig": {
@@ -132,5 +133,17 @@
     },
     "exclude_files": {
     }
+  },
+  "properpermissions": {
+    "only_files": {
+      "beacon-chain/.*": "",
+      "slasher/.*": "",
+      "shared/.*": "",
+      "validator/.*": ""
+    },
+    "exclude_files": {
+      ".*_test\\.go": "Tests are ok",
+      "shared/fileutil/fileutil.go": "Package which defines the proper rules"
+    }
   }
 }

shared/fileutil/fileutil.go

@@ -8,9 +8,8 @@ import (
 	"path/filepath"
 	"strings"
 
-	"github.com/prysmaticlabs/prysm/shared/params"
-
 	"github.com/pkg/errors"
+	"github.com/prysmaticlabs/prysm/shared/params"
 	log "github.com/sirupsen/logrus"
 )
 
@@ -28,6 +27,51 @@ func ExpandPath(p string) (string, error) {
 	return filepath.Abs(path.Clean(os.ExpandEnv(p)))
 }
 
+// MkdirAll takes in a path, expands it if necessary, and looks through the
+// permissions of every directory along the path, ensuring we are not attempting
+// to overwrite any existing permissions. Finally, creates the directory accordingly
+// with standardized, Prysm project permissions. This is the static-analysis enforced
+// method for creating a directory programmatically in Prysm.
+func MkdirAll(dirPath string) error {
+	expanded, err := ExpandPath(dirPath)
+	if err != nil {
+		return err
+	}
+	exists, err := HasDir(expanded)
+	if err != nil {
+		return err
+	}
+	if exists {
+		info, err := os.Stat(expanded)
+		if err != nil {
+			return err
+		}
+		if info.Mode().Perm() != params.BeaconIoConfig().ReadWriteExecutePermissions {
+			return errors.New("dir already exists without proper 0700 permissions")
+		}
+	}
+	return os.MkdirAll(expanded, params.BeaconIoConfig().ReadWriteExecutePermissions)
+}
+
+// WriteFile is the static-analysis enforced method for writing binary data to a file
+// in Prysm, enforcing a single entrypoint with standardized permissions.
+func WriteFile(file string, data []byte) error {
+	expanded, err := ExpandPath(file)
+	if err != nil {
+		return err
+	}
+	if FileExists(expanded) {
+		info, err := os.Stat(expanded)
+		if err != nil {
+			return err
+		}
+		if info.Mode() != params.BeaconIoConfig().ReadWritePermissions {
+			return errors.New("file already exists without proper 0600 permissions")
+		}
+	}
+	return ioutil.WriteFile(expanded, data, params.BeaconIoConfig().ReadWritePermissions)
+}
+
 // HomeDir for a user.
 func HomeDir() string {
 	if home := os.Getenv("HOME"); home != "" {

shared/fileutil/fileutil_test.go

@@ -21,6 +21,7 @@ import (
 	"io/ioutil"
 	"os"
 	"os/user"
+	"path/filepath"
 	"testing"
 
 	"github.com/prysmaticlabs/prysm/shared/fileutil"
@@ -47,6 +48,77 @@ func TestPathExpansion(t *testing.T) {
 	}
 }
 
+func TestMkdirAll_AlreadyExists_WrongPermissions(t *testing.T) {
+	dirName := testutil.TempDir() + "somedir"
+	err := os.MkdirAll(dirName, os.ModePerm)
+	require.NoError(t, err)
+	defer func() {
+		assert.NoError(t, os.RemoveAll(dirName))
+	}()
+	err = fileutil.MkdirAll(dirName)
+	assert.ErrorContains(t, "already exists without proper 0700 permissions", err)
+}
+
+func TestMkdirAll_AlreadyExists_OK(t *testing.T) {
+	dirName := testutil.TempDir() + "somedir"
+	err := os.MkdirAll(dirName, params.BeaconIoConfig().ReadWriteExecutePermissions)
+	require.NoError(t, err)
+	defer func() {
+		assert.NoError(t, os.RemoveAll(dirName))
+	}()
+	assert.NoError(t, fileutil.MkdirAll(dirName))
+}
+
+func TestMkdirAll_OK(t *testing.T) {
+	dirName := testutil.TempDir() + "somedir"
+	defer func() {
+		assert.NoError(t, os.RemoveAll(dirName))
+	}()
+	err := fileutil.MkdirAll(dirName)
+	assert.NoError(t, err)
+	exists, err := fileutil.HasDir(dirName)
+	require.NoError(t, err)
+	assert.Equal(t, true, exists)
+}
+
+func TestWriteFile_AlreadyExists_WrongPermissions(t *testing.T) {
+	dirName := testutil.TempDir() + "somedir"
+	err := os.MkdirAll(dirName, os.ModePerm)
+	require.NoError(t, err)
+	defer func() {
+		assert.NoError(t, os.RemoveAll(dirName))
+	}()
+	someFileName := filepath.Join(dirName, "somefile.txt")
+	require.NoError(t, ioutil.WriteFile(someFileName, []byte("hi"), os.ModePerm))
+	err = fileutil.WriteFile(someFileName, []byte("hi"))
+	assert.ErrorContains(t, "already exists without proper 0600 permissions", err)
+}
+
+func TestWriteFile_AlreadyExists_OK(t *testing.T) {
+	dirName := testutil.TempDir() + "somedir"
+	err := os.MkdirAll(dirName, os.ModePerm)
+	require.NoError(t, err)
+	defer func() {
+		assert.NoError(t, os.RemoveAll(dirName))
+	}()
+	someFileName := filepath.Join(dirName, "somefile.txt")
+	require.NoError(t, ioutil.WriteFile(someFileName, []byte("hi"), params.BeaconIoConfig().ReadWritePermissions))
+	assert.NoError(t, fileutil.WriteFile(someFileName, []byte("hi")))
+}
+
+func TestWriteFile_OK(t *testing.T) {
+	dirName := testutil.TempDir() + "somedir"
+	err := os.MkdirAll(dirName, os.ModePerm)
+	require.NoError(t, err)
+	defer func() {
+		assert.NoError(t, os.RemoveAll(dirName))
+	}()
+	someFileName := filepath.Join(dirName, "somefile.txt")
+	require.NoError(t, fileutil.WriteFile(someFileName, []byte("hi")))
+	exists := fileutil.FileExists(someFileName)
+	assert.Equal(t, true, exists)
+}
+
 func TestCopyFile(t *testing.T) {
 	fName := testutil.TempDir() + "testfile"
 	err := ioutil.WriteFile(fName, []byte{1, 2, 3}, params.BeaconIoConfig().ReadWritePermissions)

shared/keystore/BUILD.bazel

@@ -13,7 +13,7 @@ go_library(
     visibility = ["//visibility:public"],
     deps = [
         "//shared/bls:go_default_library",
-        "//shared/params:go_default_library",
+        "//shared/fileutil:go_default_library",
         "//shared/timeutils:go_default_library",
         "@com_github_minio_sha256_simd//:go_default_library",
         "@com_github_pborman_uuid//:go_default_library",

shared/keystore/key.go

@@ -27,7 +27,7 @@ import (
 
 	"github.com/pborman/uuid"
 	"github.com/prysmaticlabs/prysm/shared/bls"
-	"github.com/prysmaticlabs/prysm/shared/params"
+	"github.com/prysmaticlabs/prysm/shared/fileutil"
 )
 
 const (
@@ -170,7 +170,7 @@ func storeNewRandomKey(ks keyStore, password string) error {
 func writeKeyFile(file string, content []byte) error {
 	// Create the keystore directory with appropriate permissions
 	// in case it is not present yet.
-	if err := os.MkdirAll(filepath.Dir(file), params.BeaconIoConfig().ReadWriteExecutePermissions); err != nil {
+	if err := fileutil.MkdirAll(filepath.Dir(file)); err != nil {
 		return err
 	}
 	// Atomic write: create a temporary hidden file first

shared/tos/BUILD.bazel

@@ -9,7 +9,6 @@ go_library(
     deps = [
         "//shared/cmd:go_default_library",
         "//shared/fileutil:go_default_library",
-        "//shared/params:go_default_library",
         "//shared/promptutil:go_default_library",
         "@com_github_logrusorgru_aurora//:go_default_library",
         "@com_github_sirupsen_logrus//:go_default_library",