diff --git a/libs/os/os.go b/libs/os/os.go index fb55e0171..79e340d19 100644 --- a/libs/os/os.go +++ b/libs/os/os.go @@ -1,6 +1,7 @@ package os import ( + "errors" "fmt" "io" "io/ioutil" @@ -84,16 +85,19 @@ func MustWriteFile(filePath string, contents []byte, mode os.FileMode) { // CopyFile copies a file. It truncates the destination file if it exists. func CopyFile(src, dst string) error { - info, err := os.Stat(src) + srcfile, err := os.Open(src) if err != nil { return err } + defer srcfile.Close() - srcfile, err := os.Open(src) + info, err := srcfile.Stat() if err != nil { return err } - defer srcfile.Close() + if info.IsDir() { + return errors.New("cannot read from directories") + } // create new file, truncate if exists and apply same permissions as the original one dstfile, err := os.OpenFile(dst, os.O_RDWR|os.O_CREATE|os.O_TRUNC, info.Mode().Perm()) diff --git a/libs/os/os_test.go b/libs/os/os_test.go index c912465c5..88bf1412c 100644 --- a/libs/os/os_test.go +++ b/libs/os/os_test.go @@ -71,3 +71,44 @@ func TestEnsureDir(t *testing.T) { err = EnsureDir(filepath.Join(tmp, "linkfile"), 0755) require.Error(t, err) } + +// Ensure that using CopyFile does not truncate the destination file before +// the origin is positively a non-directory and that it is ready for copying. +// See https://github.com/tendermint/tendermint/issues/6427 +func TestTrickedTruncation(t *testing.T) { + tmpDir, err := ioutil.TempDir(os.TempDir(), "pwn_truncate") + if err != nil { + t.Fatal(err) + } + defer os.Remove(tmpDir) + + originalWALPath := filepath.Join(tmpDir, "wal") + originalWALContent := []byte("I AM BECOME DEATH, DESTROYER OF ALL WORLDS!") + if err := ioutil.WriteFile(originalWALPath, originalWALContent, 0755); err != nil { + t.Fatal(err) + } + + // 1. Sanity check. + readWAL, err := ioutil.ReadFile(originalWALPath) + if err != nil { + t.Fatal(err) + } + if !bytes.Equal(readWAL, originalWALContent) { + t.Fatalf("Cannot proceed as the content does not match\nGot: %q\nWant: %q", readWAL, originalWALContent) + } + + // 2. Now cause the truncation of the original file. + // It is absolutely legal to invoke os.Open on a directory. + if err := CopyFile(tmpDir, originalWALPath); err == nil { + t.Fatal("Expected an error") + } + + // 3. Check the WAL's content + reReadWAL, err := ioutil.ReadFile(originalWALPath) + if err != nil { + t.Fatal(err) + } + if !bytes.Equal(reReadWAL, originalWALContent) { + t.Fatalf("Oops, the WAL's content was changed :(\nGot: %q\nWant: %q", reReadWAL, originalWALContent) + } +}