From d2d226e8f9499ac85b9297eb40216b9768f93081 Mon Sep 17 00:00:00 2001 From: Lifubang Date: Fri, 31 Aug 2018 11:17:42 +0800 Subject: [PATCH 1/3] fix unexpected delete bug when container id is .. Signed-off-by: Lifubang --- libcontainer/factory_linux.go | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/libcontainer/factory_linux.go b/libcontainer/factory_linux.go index 4cbd70cd..14e998b4 100644 --- a/libcontainer/factory_linux.go +++ b/libcontainer/factory_linux.go @@ -229,6 +229,10 @@ func (l *LinuxFactory) Load(id string) (Container, error) { if l.Root == "" { return nil, newGenericError(fmt.Errorf("invalid root"), ConfigInvalid) } + //when load, we need to check id is valid or not. + if err := l.validateID(id); err != nil { + return nil, err + } containerRoot := filepath.Join(l.Root, id) state, err := l.loadState(containerRoot, id) if err != nil { @@ -355,7 +359,23 @@ func (l *LinuxFactory) loadState(root, id string) (*State, error) { } func (l *LinuxFactory) validateID(id string) error { - if !idRegex.MatchString(id) { + if !idRegex.MatchString(id) || id == ".." || id == "." { + return newGenericError(fmt.Errorf("invalid id format: %v", id), InvalidIdFormat) + } + + //For unforeseen invalid id situations, can checked by is SubDir? + rootPath, err := filepath.Abs(l.Root) + if err != nil { + return err + } + + containerRoot := filepath.Join(l.Root, id) + rootCheckPath, err := filepath.Abs(filepath.Join(containerRoot, "..")) + if err != nil { + return err + } + + if rootPath != rootCheckPath { return newGenericError(fmt.Errorf("invalid id format: %v", id), InvalidIdFormat) } From 4fae8fcce2ee889e4e7b21c53a64aba0f9b87e3f Mon Sep 17 00:00:00 2001 From: Lifubang Date: Mon, 3 Sep 2018 23:27:31 +0800 Subject: [PATCH 2/3] code optimization after review Signed-off-by: Lifubang --- libcontainer/factory_linux.go | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-) diff --git a/libcontainer/factory_linux.go b/libcontainer/factory_linux.go index 14e998b4..71712428 100644 --- a/libcontainer/factory_linux.go +++ b/libcontainer/factory_linux.go @@ -359,23 +359,7 @@ func (l *LinuxFactory) loadState(root, id string) (*State, error) { } func (l *LinuxFactory) validateID(id string) error { - if !idRegex.MatchString(id) || id == ".." || id == "." { - return newGenericError(fmt.Errorf("invalid id format: %v", id), InvalidIdFormat) - } - - //For unforeseen invalid id situations, can checked by is SubDir? - rootPath, err := filepath.Abs(l.Root) - if err != nil { - return err - } - - containerRoot := filepath.Join(l.Root, id) - rootCheckPath, err := filepath.Abs(filepath.Join(containerRoot, "..")) - if err != nil { - return err - } - - if rootPath != rootCheckPath { + if id == "." || !idRegex.MatchString(id) || utils.CleanPath(id) != id { return newGenericError(fmt.Errorf("invalid id format: %v", id), InvalidIdFormat) } From 4eb30fcdbeca6af165891ad3900896694e92feff Mon Sep 17 00:00:00 2001 From: Lifubang Date: Tue, 4 Sep 2018 09:02:18 +0800 Subject: [PATCH 3/3] code optimization: use securejoin.SecureJoin and CleanPath Signed-off-by: Lifubang --- libcontainer/factory_linux.go | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/libcontainer/factory_linux.go b/libcontainer/factory_linux.go index 71712428..612ccd74 100644 --- a/libcontainer/factory_linux.go +++ b/libcontainer/factory_linux.go @@ -11,6 +11,7 @@ import ( "runtime/debug" "strconv" + "github.com/cyphar/filepath-securejoin" "github.com/opencontainers/runc/libcontainer/cgroups" "github.com/opencontainers/runc/libcontainer/cgroups/fs" "github.com/opencontainers/runc/libcontainer/cgroups/systemd" @@ -195,7 +196,10 @@ func (l *LinuxFactory) Create(id string, config *configs.Config) (Container, err if err := l.Validator.Validate(config); err != nil { return nil, newGenericError(err, ConfigInvalid) } - containerRoot := filepath.Join(l.Root, id) + containerRoot, err := securejoin.SecureJoin(l.Root, id) + if err != nil { + return nil, err + } if _, err := os.Stat(containerRoot); err == nil { return nil, newGenericError(fmt.Errorf("container with id exists: %v", id), IdInUse) } else if !os.IsNotExist(err) { @@ -233,7 +237,10 @@ func (l *LinuxFactory) Load(id string) (Container, error) { if err := l.validateID(id); err != nil { return nil, err } - containerRoot := filepath.Join(l.Root, id) + containerRoot, err := securejoin.SecureJoin(l.Root, id) + if err != nil { + return nil, err + } state, err := l.loadState(containerRoot, id) if err != nil { return nil, err @@ -343,7 +350,11 @@ func (l *LinuxFactory) StartInitialization() (err error) { } func (l *LinuxFactory) loadState(root, id string) (*State, error) { - f, err := os.Open(filepath.Join(root, stateFilename)) + stateFilePath, err := securejoin.SecureJoin(root, stateFilename) + if err != nil { + return nil, err + } + f, err := os.Open(stateFilePath) if err != nil { if os.IsNotExist(err) { return nil, newGenericError(fmt.Errorf("container %q does not exist", id), ContainerNotExists) @@ -359,7 +370,7 @@ func (l *LinuxFactory) loadState(root, id string) (*State, error) { } func (l *LinuxFactory) validateID(id string) error { - if id == "." || !idRegex.MatchString(id) || utils.CleanPath(id) != id { + if !idRegex.MatchString(id) || string(os.PathSeparator)+id != utils.CleanPath(string(os.PathSeparator)+id) { return newGenericError(fmt.Errorf("invalid id format: %v", id), InvalidIdFormat) }