From 8adfbf2dc34560df7436c89b59a9749d2dd3b78e Mon Sep 17 00:00:00 2001
From: Jakob Unterwurzacher <jakobunt@gmail.com>
Date: Sun, 12 Feb 2017 15:35:50 +0100
Subject: [PATCH] Check for trailing garbage after the password

From the comment:

// CheckTrailingGarbage tries to read one byte from stdin and exits with a
// fatal error if the read returns any data.
// This is meant to be called after reading the password, when there is no more
// data expected. This helps to catch problems with third-party tools that
// interface with gocryptfs.
---
 init_dir.go                   |  1 +
 internal/readpassword/read.go | 32 +++++++++++++++++++++++++
 main.go                       |  1 +
 mount.go                      |  2 ++
 tests/cli/cli_test.go         | 44 +++++++++++++++++++++++++++++++++++
 5 files changed, 80 insertions(+)

diff --git a/init_dir.go b/init_dir.go
index 89af495..bf6740e 100644
--- a/init_dir.go
+++ b/init_dir.go
@@ -36,6 +36,7 @@ func initDir(args *argContainer) {
 		tlog.Info.Printf("Choose a password for protecting your files.")
 	}
 	password := readpassword.Twice(args.extpass)
+	readpassword.CheckTrailingGarbage()
 	creator := tlog.ProgramName + " " + GitVersion
 	err = configfile.CreateConfFile(args.config, password, args.plaintextnames, args.scryptn, creator, args.aessiv, args.raw64)
 	if err != nil {
diff --git a/internal/readpassword/read.go b/internal/readpassword/read.go
index 49cf8ef..fe9be45 100644
--- a/internal/readpassword/read.go
+++ b/internal/readpassword/read.go
@@ -7,6 +7,8 @@ import (
 	"os"
 	"os/exec"
 	"strings"
+	"sync"
+	"time"
 
 	"golang.org/x/crypto/ssh/terminal"
 
@@ -141,3 +143,33 @@ func readLineUnbuffered(r io.Reader) (l string) {
 		l = l + string(b)
 	}
 }
+
+// CheckTrailingGarbage tries to read one byte from stdin and exits with a
+// fatal error if the read returns any data.
+// This is meant to be called after reading the password, when there is no more
+// data expected. This helps to catch problems with third-party tools that
+// interface with gocryptfs.
+//
+// This is tested via TestInitTrailingGarbage() in tests/cli/cli_test.go.
+func CheckTrailingGarbage() {
+	if terminal.IsTerminal(int(os.Stdin.Fd())) {
+		// Be lenient when interacting with a human.
+		return
+	}
+	var wg sync.WaitGroup
+	wg.Add(1)
+	go func() {
+		b := make([]byte, 1)
+		wg.Done()
+		n, _ := os.Stdin.Read(b)
+		if n > 0 {
+			tlog.Fatal.Printf("Received trailing garbage after the password")
+			os.Exit(exitCode)
+		}
+	}()
+	// Wait for the goroutine to start up plus one millisecond for the read to
+	// return. If there is data available, this SHOULD be plenty of time to
+	// read one byte. However, I don't see a way to be sure.
+	wg.Wait()
+	time.Sleep(1 * time.Millisecond)
+}
diff --git a/main.go b/main.go
index bb47e46..21f2d06 100644
--- a/main.go
+++ b/main.go
@@ -87,6 +87,7 @@ func changePassword(args *argContainer) {
 	}
 	tlog.Info.Println("Please enter your new password.")
 	newPw := readpassword.Twice(args.extpass)
+	readpassword.CheckTrailingGarbage()
 	confFile.EncryptKey(masterkey, newPw, confFile.ScryptObject.LogN())
 	if args.masterkey != "" {
 		bak := args.config + ".bak"
diff --git a/mount.go b/mount.go
index cd14dd9..032589d 100644
--- a/mount.go
+++ b/mount.go
@@ -22,6 +22,7 @@ import (
 	"github.com/rfjakob/gocryptfs/internal/ctlsock"
 	"github.com/rfjakob/gocryptfs/internal/fusefrontend"
 	"github.com/rfjakob/gocryptfs/internal/fusefrontend_reverse"
+	"github.com/rfjakob/gocryptfs/internal/readpassword"
 	"github.com/rfjakob/gocryptfs/internal/tlog"
 )
 
@@ -96,6 +97,7 @@ func doMount(args *argContainer) int {
 			}
 			os.Exit(ErrExitLoadConf)
 		}
+		readpassword.CheckTrailingGarbage()
 		printMasterKey(masterkey)
 	}
 	// We cannot use JSON for pretty-printing as the fields are unexported
diff --git a/tests/cli/cli_test.go b/tests/cli/cli_test.go
index b39c816..cce7fa0 100644
--- a/tests/cli/cli_test.go
+++ b/tests/cli/cli_test.go
@@ -272,3 +272,47 @@ func TestShadows(t *testing.T) {
 		t.Errorf("Should have failed")
 	}
 }
+
+// TestInitTrailingGarbage verfies that gocryptfs exits with an error if we
+// pass additional data after the password.
+func TestInitTrailingGarbage(t *testing.T) {
+	table := []struct {
+		pw            string
+		closeStdin    bool
+		expectSuccess bool
+	}{
+		{"foo\n", false, true},
+		{"foo", true, true},
+		{"foo\n", true, true},
+		{"foo\n\n", false, false},
+		{"foo\nbar", false, false},
+		{"foo\n\n", true, false},
+		{"foo\nbar", true, false},
+	}
+	for _, row := range table {
+		dir, err := ioutil.TempDir(test_helpers.TmpDir, "")
+		if err != nil {
+			t.Fatal(err)
+		}
+		cmd := exec.Command(test_helpers.GocryptfsBinary, "-q", "-init", "-scryptn=10", dir)
+		childStdin, err := cmd.StdinPipe()
+		if err != nil {
+			t.Fatal(err)
+		}
+		err = cmd.Start()
+		if err != nil {
+			t.Fatal(err)
+		}
+		childStdin.Write([]byte(row.pw))
+		if row.closeStdin {
+			childStdin.Close()
+		}
+		err = cmd.Wait()
+		success := (err == nil)
+		if success == true && row.expectSuccess == false {
+			t.Errorf("pw=%q should have failed, but succeeded", row.pw)
+		} else if success == false && row.expectSuccess == true {
+			t.Errorf("pw=%q should have succeeded, but failed", row.pw)
+		}
+	}
+}