From 254f8ea9eafd1678e178b3292a00b8aca517a864 Mon Sep 17 00:00:00 2001 From: Wei Congrui Date: Sat, 29 Jul 2017 18:23:00 +0800 Subject: [PATCH] crypto/{aes,cipher,rc4}: fix out of bounds write in stream ciphers Functions XORKeyStream should panic if len(dst) < len(src), but it write to dst before bounds checking. In asm routines and fastXORBytes, this is an out of bounds write. Fixes #21104 Change-Id: I354346cda8d63910f3bb619416ffd54cd0a04a0b Reviewed-on: https://go-review.googlesource.com/52050 Reviewed-by: Brad Fitzpatrick Run-TryBot: Brad Fitzpatrick TryBot-Result: Gobot Gobot --- src/crypto/aes/ctr_s390x.go | 4 +++ src/crypto/cipher/xor.go | 9 ++++-- src/crypto/issue21104_test.go | 61 +++++++++++++++++++++++++++++++++++ src/crypto/rc4/rc4_asm.go | 2 ++ 4 files changed, 74 insertions(+), 2 deletions(-) create mode 100644 src/crypto/issue21104_test.go diff --git a/src/crypto/aes/ctr_s390x.go b/src/crypto/aes/ctr_s390x.go index 94dea5ccdf..8078aa6802 100644 --- a/src/crypto/aes/ctr_s390x.go +++ b/src/crypto/aes/ctr_s390x.go @@ -64,6 +64,10 @@ func (c *aesctr) refill() { } func (c *aesctr) XORKeyStream(dst, src []byte) { + if len(src) > 0 { + // Assert len(dst) >= len(src) + _ = dst[len(src)-1] + } for len(src) > 0 { if len(c.buffer) == 0 { c.refill() diff --git a/src/crypto/cipher/xor.go b/src/crypto/cipher/xor.go index 01ca0a9f08..5b26eace09 100644 --- a/src/crypto/cipher/xor.go +++ b/src/crypto/cipher/xor.go @@ -19,6 +19,11 @@ func fastXORBytes(dst, a, b []byte) int { if len(b) < n { n = len(b) } + if n == 0 { + return 0 + } + // Assert dst has enough space + _ = dst[n-1] w := n / wordSize if w > 0 { @@ -48,8 +53,8 @@ func safeXORBytes(dst, a, b []byte) int { return n } -// xorBytes xors the bytes in a and b. The destination is assumed to have enough -// space. Returns the number of bytes xor'd. +// xorBytes xors the bytes in a and b. The destination should have enough +// space, otherwise xorBytes will panic. Returns the number of bytes xor'd. func xorBytes(dst, a, b []byte) int { if supportsUnaligned { return fastXORBytes(dst, a, b) diff --git a/src/crypto/issue21104_test.go b/src/crypto/issue21104_test.go new file mode 100644 index 0000000000..b4276df4e1 --- /dev/null +++ b/src/crypto/issue21104_test.go @@ -0,0 +1,61 @@ +// Copyright 2017 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package crypto + +import ( + "crypto/aes" + "crypto/cipher" + "crypto/rc4" + "testing" +) + +func TestRC4OutOfBoundsWrite(t *testing.T) { + // This cipherText is encrypted "0123456789" + cipherText := []byte{238, 41, 187, 114, 151, 2, 107, 13, 178, 63} + cipher, err := rc4.NewCipher([]byte{0}) + if err != nil { + panic(err) + } + test(t, "RC4", cipherText, cipher.XORKeyStream) +} +func TestCTROutOfBoundsWrite(t *testing.T) { + testBlock(t, "CTR", cipher.NewCTR) +} +func TestOFBOutOfBoundsWrite(t *testing.T) { + testBlock(t, "OFB", cipher.NewOFB) +} +func TestCFBEncryptOutOfBoundsWrite(t *testing.T) { + testBlock(t, "CFB Encrypt", cipher.NewCFBEncrypter) +} +func TestCFBDecryptOutOfBoundsWrite(t *testing.T) { + testBlock(t, "CFB Decrypt", cipher.NewCFBDecrypter) +} +func testBlock(t *testing.T, name string, newCipher func(cipher.Block, []byte) cipher.Stream) { + // This cipherText is encrypted "0123456789" + cipherText := []byte{86, 216, 121, 231, 219, 191, 26, 12, 176, 117} + var iv, key [16]byte + block, err := aes.NewCipher(key[:]) + if err != nil { + panic(err) + } + stream := newCipher(block, iv[:]) + test(t, name, cipherText, stream.XORKeyStream) +} +func test(t *testing.T, name string, cipherText []byte, xor func([]byte, []byte)) { + want := "abcdefghij" + plainText := []byte(want) + shorterLen := len(cipherText) / 2 + defer func() { + err := recover() + if err == nil { + t.Errorf("%v XORKeyStream expected to panic on len(dst) < len(src), but didn't", name) + } + const plain = "0123456789" + if plainText[shorterLen] == plain[shorterLen] { + t.Errorf("%v XORKeyStream did out of bounds write, want %v, got %v", name, want, string(plainText)) + } + }() + xor(plainText[:shorterLen], cipherText) +} diff --git a/src/crypto/rc4/rc4_asm.go b/src/crypto/rc4/rc4_asm.go index 02e5b67d55..8d464547fa 100644 --- a/src/crypto/rc4/rc4_asm.go +++ b/src/crypto/rc4/rc4_asm.go @@ -14,5 +14,7 @@ func (c *Cipher) XORKeyStream(dst, src []byte) { if len(src) == 0 { return } + // Assert len(dst) >= len(src) + _ = dst[len(src)-1] xorKeyStream(&dst[0], &src[0], len(src), &c.s, &c.i, &c.j) } -- 2.48.1