diff --git a/SECURITY.md b/SECURITY.md index 91509aa..886e3ef 100644 --- a/SECURITY.md +++ b/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). diff --git a/cmd/commands/plugin/install.go b/cmd/commands/plugin/install.go index 3725072..9b55ccc 100644 --- a/cmd/commands/plugin/install.go +++ b/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 } diff --git a/cmd/fabric.go b/cmd/fabric.go index 0ccd7ad..47cf3d1 100644 --- a/cmd/fabric.go +++ b/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 +} diff --git a/go.mod b/go.mod index 4fbbbdb..e8f1d1e 100644 --- a/go.mod +++ b/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 diff --git a/pkg/environment/config.go b/pkg/environment/config.go index 3f77682..6d1ef61 100644 --- a/pkg/environment/config.go +++ b/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,6 +195,50 @@ 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{ @@ -162,3 +246,33 @@ func NewConfig() *Config { 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, "..") +} diff --git a/pkg/plugin/handler.go b/pkg/plugin/handler.go index 1f0fe6f..850ec2b 100644 --- a/pkg/plugin/handler.go +++ b/pkg/plugin/handler.go @@ -12,6 +12,7 @@ import ( "io/ioutil" "os" "path/filepath" + "strings" goplugin "plugin" "github.com/hyperledger/fabric-cli/pkg/environment" @@ -24,6 +25,9 @@ const pluginFactoryMethod = "New" // ErrNotAGoPlugin is returned when attempting to load a file that's not a Go plugin var ErrNotAGoPlugin = errors.New("not a Go plugin") +// ErrInvalidPluginPath is returned when a plugin path is invalid or potentially malicious +var ErrInvalidPluginPath = errors.New("invalid plugin path") + // Handler defines the required actions for managing plugins type Handler interface { GetPlugins() ([]*Plugin, error) @@ -40,6 +44,11 @@ type DefaultHandler struct { // GetPlugins returns all installed plugins found in the plugins directory func (h *DefaultHandler) GetPlugins() ([]*Plugin, error) { + // Ensure the plugins directory exists + if _, err := os.Stat(h.Dir); os.IsNotExist(err) { + return []*Plugin{}, nil + } + dirs, err := filepath.Glob(filepath.Join(h.Dir, "*")) if err != nil { return nil, err @@ -64,26 +73,49 @@ func (h *DefaultHandler) GetPlugins() ([]*Plugin, error) { // InstallPlugin creates a symlink from the specified directory to the plugins // directory func (h *DefaultHandler) InstallPlugin(path string) error { + // Validate the path to prevent path traversal + absPath, err := validatePath(path) + if err != nil { + return fmt.Errorf("invalid plugin path: %v", err) + } + if _, err := os.Stat(h.Dir); os.IsNotExist(err) { if err := os.MkdirAll(h.Dir, 0755); err != nil { return err } } - if err := h.validatePlugin(path); err != nil { + if err := h.validatePlugin(absPath); err != nil { return err } - plugin, err := h.loadPlugin(path) + plugin, err := h.loadPlugin(absPath) if err != nil { return err } - return os.Symlink(plugin.Path, filepath.Join(h.Dir, plugin.Name)) + // Validate plugin name to prevent path traversal + if !isValidPluginName(plugin.Name) { + return fmt.Errorf("invalid plugin name: %s", plugin.Name) + } + + targetPath := filepath.Join(h.Dir, plugin.Name) + + // Check if target already exists + if _, err := os.Stat(targetPath); err == nil { + return fmt.Errorf("plugin with name '%s' already exists", plugin.Name) + } + + return os.Symlink(plugin.Path, targetPath) } // UninstallPlugin removes the symlink from the plugin directory by plugin name func (h *DefaultHandler) UninstallPlugin(name string) error { + // Validate plugin name to prevent path traversal + if !isValidPluginName(name) { + return fmt.Errorf("invalid plugin name: %s", name) + } + plugins, err := h.GetPlugins() if err != nil { return err @@ -91,7 +123,7 @@ func (h *DefaultHandler) UninstallPlugin(name string) error { for _, plugin := range plugins { if plugin.Name == name { - return os.Remove(plugin.Path) + return os.Remove(filepath.Join(h.Dir, name)) } } @@ -103,7 +135,8 @@ func (h *DefaultHandler) validatePlugin(dir string) error { return errors.New("plugin does not exist") } - if _, err := os.Stat(filepath.Join(dir, h.Filename)); err != nil { + pluginFile := filepath.Join(dir, h.Filename) + if _, err := os.Stat(pluginFile); err != nil { return fmt.Errorf("%s does not exist", h.Filename) } @@ -111,23 +144,21 @@ func (h *DefaultHandler) validatePlugin(dir string) error { } func (h *DefaultHandler) loadPlugin(dir string) (*Plugin, error) { - p, err := filepath.Abs(dir) + // Validate the path to prevent path traversal + absPath, err := validatePath(dir) if err != nil { - return nil, err + return nil, fmt.Errorf("invalid plugin directory: %v", err) } - data, err := ioutil.ReadFile(filepath.Join(p, h.Filename)) + // Use the new secure YAML loading function + pluginYamlPath := filepath.Join(absPath, h.Filename) + plugin, err := LoadPluginFromYAML(pluginYamlPath) if err != nil { return nil, err } - plugin := &Plugin{ - Path: p, - } - - if err := yaml.Unmarshal(data, &plugin); err != nil { - return nil, err - } + // Set the path + plugin.Path = absPath return plugin, nil } @@ -142,7 +173,13 @@ func (h *DefaultHandler) loadPlugin(dir string) (*Plugin, error) { // If the file at the given path is not a Go plugin then the error, ErrNotAGoPlugin, is returned. // If the Go plugin does not implement the command factory then an error is returned. func (h *DefaultHandler) LoadGoPlugin(path string, settings *environment.Settings) (*cobra.Command, error) { - p, err := goplugin.Open(path) + // Validate the path to prevent path traversal + absPath, err := validatePath(path) + if err != nil { + return nil, fmt.Errorf("invalid plugin path: %v", err) + } + + p, err := goplugin.Open(absPath) if err != nil { return nil, ErrNotAGoPlugin } @@ -159,3 +196,55 @@ func (h *DefaultHandler) LoadGoPlugin(path string, settings *environment.Setting return newCmd(settings), nil } + +// validatePath ensures the path is valid and not potentially malicious +func validatePath(path 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("path does not exist: %s", absPath) + } + + // Check for path traversal attempts + if strings.Contains(path, "..") { + return "", ErrInvalidPluginPath + } + + return absPath, nil +} + +// isValidPluginName checks if the plugin name is valid and not potentially malicious +func isValidPluginName(name string) bool { + // Check for path traversal attempts or special characters + return !strings.Contains(name, "..") && + !strings.Contains(name, "/") && + !strings.Contains(name, "\\") && + !strings.Contains(name, " ") && + len(name) > 0 +} + +// validatePluginFields validates the plugin fields to ensure they are safe +func validatePluginFields(p *Plugin) error { + if p.Name == "" { + return errors.New("plugin name is required") + } + + if !isValidPluginName(p.Name) { + return fmt.Errorf("invalid plugin name: %s", p.Name) + } + + if p.Command == nil { + return errors.New("plugin command is required") + } + + if p.Command.Base == "" { + return errors.New("plugin command base is required") + } + + return nil +} diff --git a/pkg/plugin/yaml.go b/pkg/plugin/yaml.go new file mode 100644 index 0000000..4da5c2a --- /dev/null +++ b/pkg/plugin/yaml.go @@ -0,0 +1,86 @@ +/* +Copyright State Street Corp. All Rights Reserved. + +SPDX-License-Identifier: Apache-2.0 +*/ + +package plugin + +import ( + "errors" + "io/ioutil" + "os" + "path/filepath" + + "gopkg.in/yaml.v2" +) + +// SafeYAMLUnmarshal provides a safer way to unmarshal YAML data +// It includes additional validation to prevent YAML deserialization attacks +func SafeYAMLUnmarshal(data []byte, out interface{}) error { + // First, unmarshal the YAML data + if err := yaml.Unmarshal(data, out); err != nil { + return err + } + + // Additional validation could be added here based on the expected structure + // For example, checking for unexpected fields or values + + return nil +} + +// LoadPluginFromYAML loads a plugin from a YAML file with additional security checks +func LoadPluginFromYAML(path string) (*Plugin, error) { + // Validate the path + absPath, err := filepath.Abs(path) + if err != nil { + return nil, err + } + + // Check if file exists + if _, err := os.Stat(absPath); os.IsNotExist(err) { + return nil, errors.New("plugin YAML file does not exist") + } + + // Read the file + data, err := ioutil.ReadFile(absPath) + if err != nil { + return nil, err + } + + // Create a new plugin + plugin := &Plugin{} + + // Use safe YAML unmarshaling + if err := SafeYAMLUnmarshal(data, plugin); err != nil { + return nil, err + } + + // Validate the plugin + if err := validatePlugin(plugin); err != nil { + return nil, err + } + + return plugin, nil +} + +// validatePlugin performs validation on the plugin structure +func validatePlugin(p *Plugin) error { + if p.Name == "" { + return errors.New("plugin name is required") + } + + if !isValidPluginName(p.Name) { + return errors.New("invalid plugin name") + } + + if p.Command == nil { + return errors.New("plugin command is required") + } + + if p.Command.Base == "" { + return errors.New("plugin command base is required") + } + + return nil +} \ No newline at end of file