signal parsing - better input validation

The helper function we use for signal name mapping does not
check for negative numbers nor invalid (too-high) ones. This
can yield unexpected error messages:

   # podman kill -s -1 foo
   ERRO[0000] unknown signal "18446744073709551615"

This PR introduces a small wrapper for it that:

  1) Strips off a leading dash, allowing '-1' or '-HUP'
     as valid inputs; and
  2) Rejects numbers <1 or >64 (SIGRTMAX)

Also adds a test suite checking signal handling as well as
ensuring that invalid signals are rejected by the command line.

Fixes: #4746

Signed-off-by: Ed Santiago <santiago@redhat.com>

1 files changed, 18 insertions, 3 deletions

diff --git a/pkg/util/utils.go b/pkg/util/utils.go
index 5b4dfe9fa..f7d04c73b 100644
--- a/pkg/util/utils.go
+++ b/pkg/util/utils.go
@@ -9,6 +9,7 @@ import (
 	"strconv"
 	"strings"
 	"sync"
+	"syscall"
 	"time"
 
 	"github.com/BurntSushi/toml"
@@ -284,9 +285,7 @@ func GetImageConfig(changes []string) (ImageConfig, error) {
 			config.Labels[key] = val
 		case "STOPSIGNAL":
 			// Check the provided signal for validity.
-			// TODO: Worth checking range? ParseSignal allows
-			// negative numbers.
-			killSignal, err := signal.ParseSignal(value)
+			killSignal, err := ParseSignal(value)
 			if err != nil {
 				return ImageConfig{}, errors.Wrapf(err, "invalid change %q - KILLSIGNAL must be given a valid signal", change)
 			}
@@ -305,6 +304,22 @@ func GetImageConfig(changes []string) (ImageConfig, error) {
 	return config, nil
 }
 
+// Parse and validate a signal name or number
+func ParseSignal(rawSignal string) (syscall.Signal, error) {
+	// Strip off leading dash, to allow -1 or -HUP
+	basename := strings.TrimPrefix(rawSignal, "-")
+
+	signal, err := signal.ParseSignal(basename)
+	if err != nil {
+		return -1, err
+	}
+	// 64 is SIGRTMAX; wish we could get this from a standard Go library
+	if signal < 1 || signal > 64 {
+		return -1, errors.Errorf("valid signals are 1 through 64")
+	}
+	return signal, nil
+}
+
 // ParseIDMapping takes idmappings and subuid and subgid maps and returns a storage mapping
 func ParseIDMapping(mode namespaces.UsernsMode, UIDMapSlice, GIDMapSlice []string, subUIDMap, subGIDMap string) (*storage.IDMappingOptions, error) {
 	options := storage.IDMappingOptions{