diff options
Diffstat (limited to 'internal/nametransform')
-rw-r--r-- | internal/nametransform/longnames.go | 27 | ||||
-rw-r--r-- | internal/nametransform/names.go | 8 | ||||
-rw-r--r-- | internal/nametransform/names_test.go | 17 | ||||
-rw-r--r-- | internal/nametransform/pad16.go | 17 |
4 files changed, 55 insertions, 14 deletions
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 } |