From 6fdf665385c2f279d30c355fade2d40e6b616e66 Mon Sep 17 00:00:00 2001
From: Emmanuel T Odeke <emmanuel@orijtech.com>
Date: Fri, 7 May 2021 05:46:16 -0700
Subject: [PATCH] libs/os: avoid CopyFile truncating destination before
 checking if regular file (#6428)

This change fixes a potential exploitable vulnerability
that can cause the WAL to be consistently truncated by falsely
supplying the WAL path which would be any arbitrary dirrectory.

Fixes #6427
---
 libs/os/os.go      | 10 +++++++---
 libs/os/os_test.go | 41 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 48 insertions(+), 3 deletions(-)

diff --git a/libs/os/os.go b/libs/os/os.go
index ce5ca3fd1..f4b0f1810 100644
--- a/libs/os/os.go
+++ b/libs/os/os.go
@@ -1,6 +1,7 @@
 package os
 
 import (
+	"errors"
 	"fmt"
 	"io"
 	"os"
@@ -50,16 +51,19 @@ func FileExists(filePath string) bool {
 
 // 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 3228ed840..3a31de04a 100644
--- a/libs/os/os_test.go
+++ b/libs/os/os_test.go
@@ -135,3 +135,44 @@ func newTestProgram(t *testing.T, environVar string) (cmd *exec.Cmd, stdout *byt
 
 	return
 }
+
+// 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 := tmos.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)
+	}
+}