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.
This commit is contained in:
Jakob Unterwurzacher 2017-05-23 20:46:24 +02:00
parent 508fd9e1d6
commit e827763f2e
5 changed files with 59 additions and 21 deletions

View File

@ -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",

View File

@ -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".

View File

@ -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
}

View File

@ -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()
}
}
}

View File

@ -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
}