From 558e023e269552d3295574e7eecd87ad2fd7899d Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 22 Jun 2016 10:28:44 -0400 Subject: [PATCH] Don't try to copy files over themselves When copying a config module, make sure the full path for src and dst files don't match, and also check the inode in case we resolved a different path to the same file. Make a note about the unsafe usage of reusing a tempDir path. --- config/module/copy_dir.go | 48 +++++++++++++++++++++++++++++++++++++++ config/module/get.go | 2 ++ 2 files changed, 50 insertions(+) diff --git a/config/module/copy_dir.go b/config/module/copy_dir.go index f2ae63b77..35ae3ed08 100644 --- a/config/module/copy_dir.go +++ b/config/module/copy_dir.go @@ -5,6 +5,7 @@ import ( "os" "path/filepath" "strings" + "syscall" ) // copyDir copies the src directory contents into dst. Both directories @@ -19,6 +20,7 @@ func copyDir(dst, src string) error { if err != nil { return err } + if path == src { return nil } @@ -36,6 +38,19 @@ func copyDir(dst, src string) error { // destination with the path without the src on it. dstPath := filepath.Join(dst, path[len(src):]) + // we don't want to try and copy the same file over itself. + if path == dstPath { + return nil + } + + // We still might have the same file through a link, so check the + // inode if we can + if eq, err := sameInode(path, dstPath); eq { + return nil + } else if err != nil { + return err + } + // If we have a directory, make that subdirectory, then continue // the walk. if info.IsDir() { @@ -74,3 +89,36 @@ func copyDir(dst, src string) error { return filepath.Walk(src, walkFn) } + +// sameInode looks up the inode for paths a and b and returns if they are +// equal. On windows this will always return false. +func sameInode(a, b string) (bool, error) { + var aIno, bIno uint64 + aStat, err := os.Stat(a) + if err != nil { + if os.IsNotExist(err) { + return false, nil + } + return false, err + } + if st, ok := aStat.Sys().(*syscall.Stat_t); ok { + aIno = st.Ino + } + + bStat, err := os.Stat(b) + if err != nil { + if os.IsNotExist(err) { + return false, nil + } + return false, err + } + if st, ok := bStat.Sys().(*syscall.Stat_t); ok { + bIno = st.Ino + } + + if aIno > 0 && aIno == bIno { + return true, nil + } + + return false, nil +} diff --git a/config/module/get.go b/config/module/get.go index cba15277f..96b4a63c3 100644 --- a/config/module/get.go +++ b/config/module/get.go @@ -37,6 +37,8 @@ func GetCopy(dst, src string) error { if err != nil { return err } + // FIXME: This isn't completely safe. Creating and removing our temp path + // exposes where to race to inject files. if err := os.RemoveAll(tmpDir); err != nil { return err }