From b44da4c05f4972e19bb16a91aec2e3a0e089b516 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Thu, 14 Apr 2022 11:46:15 -0700 Subject: [PATCH 1/2] libct: validateID: stop using regexp Replace a regex with a simple for loop. Document the function. Signed-off-by: Kir Kolyshkin --- libcontainer/factory_linux.go | 44 +++++++++++++++++++++++++++++++---- 1 file changed, 40 insertions(+), 4 deletions(-) diff --git a/libcontainer/factory_linux.go b/libcontainer/factory_linux.go index cd39ca38fae..b8d2d9c287d 100644 --- a/libcontainer/factory_linux.go +++ b/libcontainer/factory_linux.go @@ -5,7 +5,6 @@ import ( "errors" "fmt" "os" - "regexp" "runtime/debug" "strconv" @@ -27,8 +26,6 @@ const ( execFifoFilename = "exec.fifo" ) -var idRegex = regexp.MustCompile(`^[\w+-\.]+$`) - // Create creates a new container with the given id inside a given state // directory (root), and returns a Container object. // @@ -260,8 +257,47 @@ func loadState(root string) (*State, error) { return state, nil } +// validateID checks if the supplied container ID is valid, returning +// the ErrInvalidID in case it is not. +// +// The format of valid ID was never formally defined, instead the code +// was modified to allow or disallow specific characters. +// +// Currently, a valid ID is a non-empty string consisting only of +// the following characters: +// - uppercase (A-Z) and lowercase (a-z) Latin letters; +// - digits (0-9); +// - underscore (_); +// - plus sign (+); +// - minus sign (-); +// - period (.). +// +// In addition, IDs that can't be used to represent a file name +// (such as . or ..) are rejected. + func validateID(id string) error { - if !idRegex.MatchString(id) || string(os.PathSeparator)+id != utils.CleanPath(string(os.PathSeparator)+id) { + if len(id) < 1 { + return ErrInvalidID + } + + // Allowed characters: 0-9 A-Z a-z _ + - . + for i := 0; i < len(id); i++ { + c := id[i] + switch { + case c >= 'a' && c <= 'z': + case c >= 'A' && c <= 'Z': + case c >= '0' && c <= '9': + case c == '_': + case c == '+': + case c == '-': + case c == '.': + default: + return ErrInvalidID + } + + } + + if string(os.PathSeparator)+id != utils.CleanPath(string(os.PathSeparator)+id) { return ErrInvalidID } From 0ac98807c3d058f2bccd7f4ac7abdd1dbddbdbf7 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Thu, 14 Apr 2022 12:19:39 -0700 Subject: [PATCH 2/2] libct/cg/sd: stop using regex, fix systemdVersionAtoi Rewrite systemdVersionAtoi to not use regexp, and fix two issues: 1. It was returning 0 (rather than -1) for some errors. 2. The comment was saying that the input string is without quotes, while in fact it is. Note the new function, similar to the old one, works on input either with or without quotes. Amend the test to add test cases without quotes. Signed-off-by: Kir Kolyshkin --- libcontainer/cgroups/systemd/common.go | 27 +++++++++++--------- libcontainer/cgroups/systemd/systemd_test.go | 7 +++-- 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/libcontainer/cgroups/systemd/common.go b/libcontainer/cgroups/systemd/common.go index 0c3562e6bfe..2f0767b2fd8 100644 --- a/libcontainer/cgroups/systemd/common.go +++ b/libcontainer/cgroups/systemd/common.go @@ -6,7 +6,6 @@ import ( "fmt" "math" "os" - "regexp" "strconv" "strings" "sync" @@ -231,18 +230,22 @@ func systemdVersion(cm *dbusConnManager) int { return version } -func systemdVersionAtoi(verStr string) (int, error) { - // verStr should be of the form: - // "v245.4-1.fc32", "245", "v245-1.fc32", "245-1.fc32" (without quotes). - // The result for all of the above should be 245. - // Thus, we unconditionally remove the "v" prefix - // and then match on the first integer we can grab. - re := regexp.MustCompile(`v?([0-9]+)`) - matches := re.FindStringSubmatch(verStr) - if len(matches) < 2 { - return 0, fmt.Errorf("can't parse version %s: incorrect number of matches %v", verStr, matches) +// systemdVersionAtoi extracts a numeric systemd version from the argument. +// The argument should be of the form: "v245.4-1.fc32", "245", "v245-1.fc32", +// "245-1.fc32" (with or without quotes). The result for all of the above +// should be 245. +func systemdVersionAtoi(str string) (int, error) { + // Unconditionally remove the leading prefix ("v). + str = strings.TrimLeft(str, `"v`) + // Match on the first integer we can grab. + for i := 0; i < len(str); i++ { + if str[i] < '0' || str[i] > '9' { + // First non-digit: cut the tail. + str = str[:i] + break + } } - ver, err := strconv.Atoi(matches[1]) + ver, err := strconv.Atoi(str) if err != nil { return -1, fmt.Errorf("can't parse version: %w", err) } diff --git a/libcontainer/cgroups/systemd/systemd_test.go b/libcontainer/cgroups/systemd/systemd_test.go index d280a3090aa..40584f78eba 100644 --- a/libcontainer/cgroups/systemd/systemd_test.go +++ b/libcontainer/cgroups/systemd/systemd_test.go @@ -35,8 +35,11 @@ func TestSystemdVersion(t *testing.T) { {`"v245.4-1.fc32"`, 245, false}, {`"241-1"`, 241, false}, {`"v241-1"`, 241, false}, - {"NaN", 0, true}, - {"", 0, true}, + {`333.45"`, 333, false}, + {`v321-0`, 321, false}, + {"NaN", -1, true}, + {"", -1, true}, + {"v", -1, true}, } for _, sdTest := range systemdVersionTests { ver, err := systemdVersionAtoi(sdTest.verStr)