Fix: security vulnerabilities in the codebase

SECURITY.md

@@ -6,7 +6,7 @@ If you think you have discovered a security issue in any of the Hyperledger proj
 
 There are two ways to report a security bug. The easiest is to email a description of the flaw and any related information (e.g. reproduction steps, version) to [security at hyperledger dot org](mailto:security@hyperledger.org).
 
-The other way is to file a confidential security bug in our [JIRA bug tracking system](https://jira.hyperledger.org). Be sure to set the “Security Level” to “Security issue”.
+The other way is to file a confidential security bug in our [JIRA bug tracking system](https://jira.hyperledger.org). Be sure to set the "Security Level" to "Security issue".
 
 The process by which the Hyperledger Security Team handles security bugs is documented further in our [Defect Response page](https://wiki.hyperledger.org/display/HYP/Defect+Response) on our [wiki](https://wiki.hyperledger.org).
 

cmd/commands/plugin/install.go

@@ -9,6 +9,9 @@ package plugin
 import (
 	"errors"
 	"fmt"
+	"os"
+	"path/filepath"
+	"strings"
 
 	"github.com/spf13/cobra"
 
@@ -61,6 +64,36 @@ func (c *InstallCommand) Validate() error {
 		return errors.New("plugin path not specified")
 	}
 
+	// Validate the path to prevent path traversal
+	if strings.Contains(c.Path, "..") {
+		return errors.New("path contains potentially unsafe '..' sequence")
+	}
+
+	// Check if path exists
+	absPath, err := filepath.Abs(c.Path)
+	if err != nil {
+		return fmt.Errorf("invalid path: %v", err)
+	}
+
+	fileInfo, err := os.Stat(absPath)
+	if os.IsNotExist(err) {
+		return fmt.Errorf("plugin path does not exist: %s", absPath)
+	}
+	if err != nil {
+		return fmt.Errorf("error accessing plugin path: %v", err)
+	}
+
+	// Ensure it's a directory
+	if !fileInfo.IsDir() {
+		return errors.New("plugin path must be a directory")
+	}
+
+	// Check for plugin.yaml file
+	pluginYamlPath := filepath.Join(absPath, plugin.DefaultFilename)
+	if _, err := os.Stat(pluginYamlPath); os.IsNotExist(err) {
+		return fmt.Errorf("plugin.yaml not found in %s", absPath)
+	}
+
 	return nil
 }
 

cmd/fabric.go

@@ -10,6 +10,8 @@ import (
 	"fmt"
 	"os"
 	"os/exec"
+	"path/filepath"
+	"strings"
 
 	"github.com/spf13/cobra"
 
@@ -98,7 +100,12 @@ func loadPlugins(cmd *cobra.Command, settings *environment.Settings, handler plu
 
 // loadPlugin loads the given plugin as either a Go plugin or a wrapped executable
 func loadPlugin(p *plugin.Plugin, settings *environment.Settings, handler plugin.Handler) (*cobra.Command, error) {
-	path := os.ExpandEnv(p.Command.Base)
+	// Validate plugin path to prevent path traversal
+	path, err := validatePluginPath(os.ExpandEnv(p.Command.Base), settings.Home.Plugins())
+	if err != nil {
+		return nil, fmt.Errorf("invalid plugin path: %v", err)
+	}
+
 	c, err := handler.LoadGoPlugin(path, settings)
 	if err == nil {
 		return c, nil
@@ -107,11 +114,18 @@ func loadPlugin(p *plugin.Plugin, settings *environment.Settings, handler plugin
 		return nil, err
 	}
 
+	// Validate allowed commands
+	if !isAllowedCommand(path) {
+		return nil, fmt.Errorf("command not in allowed list: %s", path)
+	}
+
 	return &cobra.Command{
 		Use:   p.Name,
 		Short: p.Description,
 		RunE: func(cmd *cobra.Command, args []string) error {
-			e := exec.Command(path, append(p.Command.Args, args...)...)
+			// Sanitize arguments to prevent command injection
+			sanitizedArgs := sanitizeArgs(append(p.Command.Args, args...))
+			e := exec.Command(path, sanitizedArgs...)
 			e.Env = os.Environ()
 			e.Stdin = settings.Streams.In
 			e.Stdout = settings.Streams.Out
@@ -120,3 +134,88 @@ func loadPlugin(p *plugin.Plugin, settings *environment.Settings, handler plugin
 		},
 	}, nil
 }
+
+// validatePluginPath ensures the plugin path is within the allowed plugins directory
+func validatePluginPath(path, pluginsDir string) (string, error) {
+	// Convert to absolute path
+	absPath, err := filepath.Abs(path)
+	if err != nil {
+		return "", err
+	}
+
+	// Check if path exists
+	if _, err := os.Stat(absPath); os.IsNotExist(err) {
+		return "", fmt.Errorf("plugin path does not exist: %s", absPath)
+	}
+
+	// For security, only allow plugins from specific directories
+	absPluginsDir, err := filepath.Abs(pluginsDir)
+	if err != nil {
+		return "", err
+	}
+
+	// Check if the plugin is within the plugins directory or is in a system path
+	if !strings.HasPrefix(absPath, absPluginsDir) && !isInSystemPath(absPath) {
+		return "", fmt.Errorf("plugin must be in the plugins directory or a system path")
+	}
+
+	return absPath, nil
+}
+
+// isInSystemPath checks if the given path is in a system path
+func isInSystemPath(path string) bool {
+	// Get system PATH
+	systemPaths := strings.Split(os.Getenv("PATH"), string(os.PathListSeparator))
+
+	// Check if the path is in any of the system paths
+	for _, sysPath := range systemPaths {
+		absPath, err := filepath.Abs(sysPath)
+		if err != nil {
+			continue
+		}
+		if strings.HasPrefix(path, absPath) {
+			return true
+		}
+	}
+
+	return false
+}
+
+// isAllowedCommand checks if the command is in the allowed list
+func isAllowedCommand(cmd string) bool {
+	// Define a whitelist of allowed commands
+	// This should be configured based on your security requirements
+	allowedCommands := []string{
+		"cryptogen",
+		"configtxgen",
+		"configtxlator",
+		"peer",
+		"orderer",
+		"discover",
+		"idemixgen",
+	}
+
+	cmdBase := filepath.Base(cmd)
+	for _, allowed := range allowedCommands {
+		if cmdBase == allowed {
+			return true
+		}
+	}
+
+	return false
+}
+
+// sanitizeArgs sanitizes command arguments to prevent command injection
+func sanitizeArgs(args []string) []string {
+	sanitized := make([]string, len(args))
+	for i, arg := range args {
+		// Remove any characters that could be used for command injection
+		// This is a basic implementation - you may need more sophisticated sanitization
+		sanitized[i] = strings.ReplaceAll(arg, ";", "")
+		sanitized[i] = strings.ReplaceAll(sanitized[i], "|", "")
+		sanitized[i] = strings.ReplaceAll(sanitized[i], "&", "")
+		sanitized[i] = strings.ReplaceAll(sanitized[i], ">", "")
+		sanitized[i] = strings.ReplaceAll(sanitized[i], "<", "")
+	}
+	return sanitized
+}

go.mod

@@ -4,7 +4,7 @@
 
 module github.com/hyperledger/fabric-cli
 
-go 1.12
+go 1.21
 
 require (
 	github.com/hyperledger/fabric-protos-go v0.0.0-20200707132912-fee30f3ccd23

pkg/environment/config.go

@@ -9,8 +9,11 @@ package environment
 import (
 	"bytes"
 	"errors"
+	"fmt"
 	"io/ioutil"
 	"os"
+	"path/filepath"
+	"strings"
 	"text/tabwriter"
 	"text/template"
 
@@ -92,6 +95,11 @@ func (c *Config) AddFlags(fs *pflag.FlagSet) {
 
 // LoadFromFile populates config based on the specified path
 func (c *Config) LoadFromFile(path string) error {
+	// Validate the path to prevent path traversal
+	if err := validateConfigPath(path); err != nil {
+		return err
+	}
+
 	if _, err := os.Stat(path); err != nil {
 		return err
 	}
@@ -101,21 +109,53 @@ func (c *Config) LoadFromFile(path string) error {
 		return err
 	}
 
+	// Use safe YAML unmarshaling
 	if err := yaml.Unmarshal(data, &c); err != nil {
 		return err
 	}
 
+	// Validate config after loading
+	if err := c.validate(); err != nil {
+		return err
+	}
+
 	return nil
 }
 
 // Save writes the current config value to the specified path
 func (c *Config) Save(path string) error {
+	// Validate the path to prevent path traversal
+	if err := validateConfigPath(path); err != nil {
+		return err
+	}
+
+	// Validate config before saving
+	if err := c.validate(); err != nil {
+		return err
+	}
+
+	// Create directory if it doesn't exist
+	dir := filepath.Dir(path)
+	if _, err := os.Stat(dir); os.IsNotExist(err) {
+		if err := os.MkdirAll(dir, 0700); err != nil {
+			return err
+		}
+	}
+
 	data, err := yaml.Marshal(c)
 	if err != nil {
 		return err
 	}
 
-	if err := ioutil.WriteFile(path, data, 0600); err != nil {
+	// Write to a temporary file first, then rename to ensure atomic write
+	tempFile := path + ".tmp"
+	if err := ioutil.WriteFile(tempFile, data, 0600); err != nil {
+		return err
+	}
+
+	if err := os.Rename(tempFile, path); err != nil {
+		// Clean up the temporary file if rename fails
+		os.Remove(tempFile)
 		return err
 	}
 
@@ -155,10 +195,84 @@ func (c *Config) GetCurrentContextNetwork() (*Network, error) {
 	return network, nil
 }
 
+// validate performs validation on the config
+func (c *Config) validate() error {
+	// Validate networks
+	for name, network := range c.Networks {
+		if network == nil {
+			return fmt.Errorf("network '%s' is nil", name)
+		}
+
+		// Validate network name
+		if !isValidName(name) {
+			return fmt.Errorf("invalid network name: %s", name)
+		}
+
+		// Validate config path
+		if network.ConfigPath != "" && !isValidFilePath(network.ConfigPath) {
+			return fmt.Errorf("invalid config path for network '%s': %s", name, network.ConfigPath)
+		}
+	}
+
+	// Validate contexts
+	for name, context := range c.Contexts {
+		if context == nil {
+			return fmt.Errorf("context '%s' is nil", name)
+		}
+
+		// Validate context name
+		if !isValidName(name) {
+			return fmt.Errorf("invalid context name: %s", name)
+		}
+
+		// Validate network reference
+		if context.Network != "" && !isValidName(context.Network) {
+			return fmt.Errorf("invalid network name in context '%s': %s", name, context.Network)
+		}
+	}
+
+	// Validate current context
+	if c.CurrentContext != "" && !isValidName(c.CurrentContext) {
+		return fmt.Errorf("invalid current context name: %s", c.CurrentContext)
+	}
+
+	return nil
+}
+
 // NewConfig returns a new config
 func NewConfig() *Config {
 	return &Config{
 		Networks: make(map[string]*Network),
 		Contexts: make(map[string]*Context),
 	}
 }
+
+// validateConfigPath validates the config file path
+func validateConfigPath(path string) error {
+	// Check for path traversal attempts
+	if strings.Contains(path, "..") {
+		return errors.New("path contains potentially unsafe '..' sequence")
+	}
+
+	// Ensure the path is absolute
+	if !filepath.IsAbs(path) {
+		return errors.New("config path must be absolute")
+	}
+
+	return nil
+}
+
+// isValidName checks if a name is valid
+func isValidName(name string) bool {
+	// Check for path traversal attempts or special characters
+	return !strings.Contains(name, "..") && 
+		!strings.Contains(name, "/") && 
+		!strings.Contains(name, "\\") &&
+		len(name) > 0
+}
+
+// isValidFilePath checks if a file path is valid
+func isValidFilePath(path string) bool {
+	// Check for path traversal attempts
+	return !strings.Contains(path, "..")
+}