nametransform: harden name decryption against invalid input

This fixes a few issues I have found reviewing the code:

1) Limit the amount of data ReadLongName() will read. Previously,
you could send gocryptfs into out-of-memory by symlinking
gocryptfs.diriv to /dev/zero.

2) Handle the empty input case in unPad16() by returning an
error. Previously, it would panic with an out-of-bounds array
read. It is unclear to me if this could actually be triggered.

3) Reject empty names after base64-decoding in DecryptName().
An empty name crashes emeCipher.Decrypt().
It is unclear to me if B64.DecodeString() can actually return
a non-error empty result, but let's guard against it anyway.

5 files changed, 59 insertions, 21 deletions

diff --git a/internal/fusefrontend/fs_dir.go b/internal/fusefrontend/fs_dir.go
index 7d1e3ef..30e715a 100644
--- a/internal/fusefrontend/fs_dir.go
+++ b/internal/fusefrontend/fs_dir.go
@@ -273,16 +273,14 @@ func (fs *FS) OpenDir(dirName string, context *fuse.Context) ([]fuse.DirEntry, f
 			// silently ignore "gocryptfs.conf" in the top level dir
 			continue
 		}
-		if !fs.args.PlaintextNames && cName == nametransform.DirIVFilename {
-			// silently ignore "gocryptfs.diriv" everywhere if dirIV is enabled
-			continue
-		}
-
 		if fs.args.PlaintextNames {
 			plain = append(plain, cipherEntries[i])
 			continue
 		}
-
+		if cName == nametransform.DirIVFilename {
+			// silently ignore "gocryptfs.diriv" everywhere if dirIV is enabled
+			continue
+		}
 		// Handle long file name
 		isLong := nametransform.LongNameNone
 		if fs.args.LongNames {
@@ -301,7 +299,6 @@ func (fs *FS) OpenDir(dirName string, context *fuse.Context) ([]fuse.DirEntry, f
 			// ignore "gocryptfs.longname.*.name"
 			continue
 		}
-
 		name, err := fs.nameTransform.DecryptName(cName, cachedIV)
 		if err != nil {
 			tlog.Warn.Printf("OpenDir %q: invalid entry %q: %v",
diff --git a/internal/nametransform/longnames.go b/internal/nametransform/longnames.go
index 54095a4..8af191d 100644
--- a/internal/nametransform/longnames.go
+++ b/internal/nametransform/longnames.go
@@ -2,7 +2,8 @@ package nametransform
 
 import (
 	"crypto/sha256"
-	"io/ioutil"
+	"fmt"
+	"io"
 	"os"
 	"path/filepath"
 	"strings"
@@ -62,13 +63,29 @@ func IsLongContent(cName string) bool {
 	return NameType(cName) == LongNameContent
 }
 
-// ReadLongName - read path.name
+// ReadLongName - read "$path.name"
 func ReadLongName(path string) (string, error) {
-	content, err := ioutil.ReadFile(path + LongNameSuffix)
+	path += LongNameSuffix
+	fd, err := os.Open(path)
 	if err != nil {
-		tlog.Warn.Printf("ReadLongName: %v", err)
+		return "", err
 	}
-	return string(content), err
+	defer fd.Close()
+	// 256 (=255 padded to 16) bytes base64-encoded take 344 bytes: "AAAAAAA...AAA=="
+	lim := 344
+	// Allocate a bigger buffer so we see whether the file is too big
+	buf := make([]byte, lim+1)
+	n, err := fd.ReadAt(buf, 0)
+	if err != nil && err != io.EOF {
+		return "", err
+	}
+	if n == 0 {
+		return "", fmt.Errorf("ReadLongName: empty file")
+	}
+	if n > lim {
+		return "", fmt.Errorf("ReadLongName: size=%d > limit=%d", n, lim)
+	}
+	return string(buf[0:n]), nil
 }
 
 // DeleteLongName deletes "hashName.name".
diff --git a/internal/nametransform/names.go b/internal/nametransform/names.go
index c96f7ce..3447583 100644
--- a/internal/nametransform/names.go
+++ b/internal/nametransform/names.go
@@ -42,6 +42,10 @@ func (n *NameTransform) DecryptName(cipherName string, iv []byte) (string, error
 	if err != nil {
 		return "", err
 	}
+	if len(bin) == 0 {
+		tlog.Warn.Printf("DecryptName: empty input")
+		return "", syscall.EBADMSG
+	}
 	if len(bin)%aes.BlockSize != 0 {
 		tlog.Debug.Printf("DecryptName %q: decoded length %d is not a multiple of 16", cipherName, len(bin))
 		return "", syscall.EBADMSG
@@ -49,14 +53,14 @@ func (n *NameTransform) DecryptName(cipherName string, iv []byte) (string, error
 	bin = n.emeCipher.Decrypt(iv, bin)
 	bin, err = unPad16(bin)
 	if err != nil {
-		tlog.Debug.Printf("DecryptName: unPad16 error: %v", err)
+		tlog.Debug.Printf("DecryptName: unPad16 error detail: %v", err)
 		// unPad16 returns detailed errors including the position of the
 		// incorrect bytes. Kill the padding oracle by lumping everything into
 		// a generic error.
 		return "", syscall.EBADMSG
 	}
 	// A name can never contain a null byte or "/". Make sure we never return those
-	// to the user, even when we read a corrupted (or fuzzed) filesystem.
+	// to the kernel, even when we read a corrupted (or fuzzed) filesystem.
 	if bytes.Contains(bin, []byte{0}) || bytes.Contains(bin, []byte("/")) {
 		return "", syscall.EBADMSG
 	}
diff --git a/internal/nametransform/names_test.go b/internal/nametransform/names_test.go
index d772af2..0254777 100644
--- a/internal/nametransform/names_test.go
+++ b/internal/nametransform/names_test.go
@@ -32,3 +32,20 @@ func TestPad16(t *testing.T) {
 		}
 	}
 }
+
+// TestUnpad16Garbage - unPad16 should never crash on corrupt or malicious inputs
+func TestUnpad16Garbage(t *testing.T) {
+	var testCases [][]byte
+	testCases = append(testCases, make([]byte, 0))
+	testCases = append(testCases, make([]byte, 16))
+	testCases = append(testCases, make([]byte, 1))
+	testCases = append(testCases, make([]byte, 17))
+	testCases = append(testCases, bytes.Repeat([]byte{16}, 16))
+	testCases = append(testCases, bytes.Repeat([]byte{17}, 16))
+	for _, v := range testCases {
+		_, err := unPad16([]byte(v))
+		if err == nil {
+			t.Fail()
+		}
+	}
+}
diff --git a/internal/nametransform/pad16.go b/internal/nametransform/pad16.go
index 2b30bcb..833be0e 100644
--- a/internal/nametransform/pad16.go
+++ b/internal/nametransform/pad16.go
@@ -31,6 +31,9 @@ func pad16(orig []byte) (padded []byte) {
 // unPad16 - remove padding
 func unPad16(padded []byte) ([]byte, error) {
 	oldLen := len(padded)
+	if oldLen == 0 {
+		return nil, errors.New("Empty input")
+	}
 	if oldLen%aes.BlockSize != 0 {
 		return nil, errors.New("Unaligned size")
 	}
@@ -39,12 +42,16 @@ func unPad16(padded []byte) ([]byte, error) {
 	// The padding byte's value is the padding length
 	padLen := int(padByte)
 	// Padding must be at least 1 byte
-	if padLen <= 0 {
+	if padLen == 0 {
 		return nil, errors.New("Padding cannot be zero-length")
 	}
-	// Larger paddings make no sense
+	// Padding more than 16 bytes make no sense
 	if padLen > aes.BlockSize {
-		return nil, fmt.Errorf("Padding too long, padLen = %d > 16", padLen)
+		return nil, fmt.Errorf("Padding too long, padLen=%d > 16", padLen)
+	}
+	// Padding cannot be as long as (or longer than) the whole string,
+	if padLen >= oldLen {
+		return nil, fmt.Errorf("Padding too long, oldLen=%d >= padLen=%d", oldLen, padLen)
 	}
 	// All padding bytes must be identical
 	for i := oldLen - padLen; i < oldLen; i++ {
@@ -53,9 +60,5 @@ func unPad16(padded []byte) ([]byte, error) {
 		}
 	}
 	newLen := oldLen - padLen
-	// Padding an empty string makes no sense
-	if newLen == 0 {
-		return nil, errors.New("Unpadded length is zero")
-	}
 	return padded[0:newLen], nil
 }